flypig.co.uk

I've moved on to the fourth in my list of tasks related to Jolla's test pages today. Fixing the double-tap action:

1. Single select widget.
2. External links.
3. Full screen.
4. Double-tap.

Once I've completed this last of the tasks, I'll be able to tick off the "Everything on the browser test page" item in the browser functionality testing issue.

The problem with double-tap can be experienced through the double tap handling and zooming page. There are various elements on the page that should behave in different ways. In order to test them most effectively the "User-scalable" option at the top of the page should be set to "yes". This allows the page to be zoomed in and out.

Then double-tapping on the coloured boxes should cause the browser to zoom in on the element, or back out again depending on how its currently framed. This part of the functinality is working correctly.

There's a "Single click" button which, when tapped should light up. When double-tapped the browser should zoom in to its position. This is also working correctly.

Finally the "Double click" button at the end has slightly different properties. When single-tapped it should do nothing. When double-tapped it should light up. So no zooming in this case.

It's this last item which is behaving incorrectly. Currently if you double-tap on the item the page will zoom in on it and there will be no lighting up. So my task today is to figure out why it's acting like that and how to fix it so that double-tapping lights up the button without zooming.

The first thing I want to do is find out where the double-tap-to-zoom functionality lives in the code. In Mozilla parlance this sort of functionality comes under something that's sometimes refered to as "APZ", short for Asynchronous Panning and Zooming. The abbreviation APZC appears in the code a lot, meaning the Asynchronous Panning and Zooming Controller. For a long time I felt quite intimidated by these terms, until I realised they described a user experience rather than a set of technical capabilities. The aim is to make the browser feel responsive.

To find the appropraite APZ code we're going to start our journey in embedhelper.js since this is Sailfish-specific, lives close to the surface in the front-end code and ties in to both the double-tap and tap-to-zoom functionalities, as we can see in this code taken from the file:

  receiveMessage: function receiveMessage(aMessage) {
    switch (aMessage.name) {
[...]
      case "Gesture:DoubleTap": {
        try {
          let [x, y] = [aMessage.json.x, aMessage.json.y];
          this._sendMouseEvent("mousemove", content, x, y);
          this._sendMouseEvent("mousedown", content, x, y, 2);
          this._sendMouseEvent("mouseup",   content, x, y);
        } catch(e) {
          Cu.reportError(e);
        }
        this._cancelTapHighlight();
        break;
      }
[...]
      case "embedui:zoomToRect": {
        if (aMessage.data) {
          let winId = Services.embedlite.getIDByWindow(content);
          // This is a hackish way as zoomToRect does not work if x-value has 
    not changed or viewport has not been scaled (zoom animation).
          // Thus, we're missing animation when viewport has not been scaled.
          let scroll = this._viewportData && 
    this._viewportData.cssCompositedRect.width === aMessage.data.width;

          if (scroll) {
            content.scrollTo(aMessage.data.x, aMessage.data.y);
          } else {
            Services.embedlite.zoomToRect(winId, aMessage.data.x, 
    aMessage.data.y, aMessage.data.width, aMessage.data.height);
          }
        }
        break;
      }
      case "embedui:scrollTo": {
        if (aMessage.data) {
            content.scrollTo(aMessage.data.x, aMessage.data.y);
        }
        break;
      }

I've added some code to the top of this function to display the message that's received, like this:

    dump("TAP: Message: " + aMessage.name + "\n");

The idea is that when I tap, long tap, double tap and so on, the actions should appear in the console output. In practice, only single tap seems to do anything:

[...]
[LWP 1025 exited]
[New LWP 7704]
TAP: Message: Gesture:SingleTap
[...]

Nevertheless this still gives me some useful material to go on because the code that's triggering this must also be sending out the Gesture:SingleTap message, so searching for this should bring me closer to the source of the problem. That brings me to the EmbedLiteViewChild::RecvHandleDoubleTap() method, which looks like this (in abridged form):

mozilla::ipc::IPCResult EmbedLiteViewChild::RecvHandleDoubleTap(const 
    LayoutDevicePoint &aPoint,
                                                                const Modifiers 
    &aModifiers,
                                                                const 
    ScrollableLayerGuid &aGuid,
                                                                const uint64_t 
    &aInputBlockId)
{
[...]
  // Check whether the element is interested in double clicks
  bool doubleclick = false;
  if (StaticPrefs::embedlite_azpc_json_doubletap()) {
[...]
    if (EventTarget* target = hittest.GetDOMEventTarget()) {
      if (nsCOMPtr<nsIContent> targetContent = do_QueryInterface(target)) {
        // Check if the element or any parent element has a double click handler
        for (Element* element = targetContent->GetAsElementOrParentElement();
             element && !doubleclick; element = element->GetParentElement()) {
          doubleclick = ElementSupportsDoubleClick(element);
        }
      }
    }
  }

  if (nsLayoutUtils::AllowZoomingForDocument(document) && !doubleclick) {
[...]
    if (APZCCallbackHelper::GetOrCreateScrollIdentifiers(
        document->GetDocumentElement(), &presShellId, &viewId)) {
      ZoomToRect(presShellId, viewId, zoomTarget);
    }
  } else {
[...]
    mHelper->DispatchMessageManagerMessage(u&quot;Gesture:DoubleTap&quot;_ns, 
    data);
  }

  return IPC_OK();
}

Since this is C++ code we can step through it using the debugger to find out what's going on:

Thread 10 "GeckoWorkerThre" hit Breakpoint 1, mozilla::embedlite::
    EmbedLiteViewChild::RecvHandleDoubleTap (this=0x7fb8d56d20, aPoint=..., 
    aModifiers=@0x7fde776594: 0, aGuid=..., aInputBlockId=@0x7fde7765a0: 14)
    at mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:966
966     {
(gdb) n
967       bool ok = false;
(gdb) n
969       NS_ENSURE_TRUE(ok, IPC_OK());
(gdb) n
971       nsIContent* content = nsLayoutUtils::FindContentFor(aGuid.mScrollId);
(gdb) n
972       NS_ENSURE_TRUE(content, IPC_OK());
(gdb) n
974       PresShell* presShell = APZCCallbackHelper::
    GetRootContentDocumentPresShellForContent(content);
(gdb) n
975       NS_ENSURE_TRUE(presShell, IPC_OK());
(gdb) n
977       RefPtr<Document> document = presShell->GetDocument();
(gdb) n
978       NS_ENSURE_TRUE(document && !document->Fullscreen(), IPC_OK());
(gdb) n
980       nsPoint offset;
(gdb) n
981       nsCOMPtr<nsIWidget> widget = mHelper->GetWidget(&offset);
(gdb) n
985       if (StaticPrefs::embedlite_azpc_json_doubletap()) {
(gdb) n
1006            presShell->GetPresContext()->CSSToDevPixelScale());
(gdb) 

What we see is that execution reaches the point where embedlite_azpc_json_doubletap() is called and then jumps straight past it. In other words embedlite_azpc_json_doubletap() must be returning false. Why is that a problem? Well, looking at the code, if this portion of code is skipped the doubleclick variable will always be set to false and then in the next condition it means ZoomToRect() will always be called instead of the code that dispatches the Gesture:DoubleTap message.

The call to embedlite_azpc_json_doubletap() is returning the value taken by the embedlite.azpc.json.doubletap static preference, so what we need is for this preference to be set to true.

Before going any further into this I can test it by setting the preference using about:config while the browser is running. Once it's set to true I find that the double-tap and tap-to-zoom both now work as expected, with the former working on the item that supports it, but the latter happening everywhere else.

So now I need to find out why embedlite.azpc.json.doubletap is set incorrectly. You may recall that I had to make changes to the way the preferences were working back on Day 97. That feels like a long time ago now! Back then I converted the preferences into static preferences, which meant stipulating them in the StaticPrefList.yaml file, along with their default values. Here's what I put in for the embedlite.azpc.json.doubletap preferences:

-   name: embedlite.azpc.json.doubletap
    type: bool
    value: false
    mirror: always

As you can see, the default value here is false. So the fix should be easy: I can just switch this default value to true. But I'd also like to understand why I got it wrong, and for this I'll need to dig back into my own changes from back in December last year. Here's the git blame for the relevant lines:

$ git blame modules/libpref/init/StaticPrefList.yaml -L 4015,4018
e035d6ff3a785 (David Llewellyn-Jones 2023-12-02 22:53:02 +0000 4015)
     -   name: embedlite.azpc.json.doubletap
e035d6ff3a785 (David Llewellyn-Jones 2023-12-02 22:53:02 +0000 4016)
         type: bool
e035d6ff3a785 (David Llewellyn-Jones 2023-12-02 22:53:02 +0000 4017)
         value: false
e035d6ff3a785 (David Llewellyn-Jones 2023-12-02 22:53:02 +0000 4018)
         mirror: always

If I check the log for the e035d6ff3a785 commit that introduced these lines, here's what it has to say:

$ git log -1 e035d6ff3a785
commit e035d6ff3a78575e6516025fac3b4e377db33133
Author: David Llewellyn-Jones <david@flypig.co.uk>
Date:   Sat Dec 2 22:53:02 2023 +0000

    Add embedlite static prefs
    
    Adds the various embedlite static preferences to StaticPrefList.yaml so
    that these will be availble for use as static pref variables in the C++
    code.

That's all very well, but this still doesn't tell me why I got the default value wrong. What was it set to in ESR 78? Well, the interesting thing is that the default value is actually stipulated twice in the ESR 78 code. First there's the code that sets the preference value in EmbedLiteViewChild.cpp. As we can see, this sets the default value to false, which is what I must have copied over when moving the preferences into StaticPrefList.yaml:

  Preferences::AddBoolVarCache(&sPostAZPCAsJson.doubleTap, 
    "embedlite.azpc.json.doubletap", false);

However, it's also set in the embedding.js file, and in tis case it's set to true:

// AZPC overrides, see EmbedLiteViewChild.cpp
pref("embedlite.azpc.handle.viewport", true);
pref("embedlite.azpc.handle.singletap", false);
pref("embedlite.azpc.handle.longtap", false);
pref("embedlite.azpc.handle.scroll", true);
pref("embedlite.azpc.json.viewport", true);
pref("embedlite.azpc.json.singletap", true);
pref("embedlite.azpc.json.doubletap", true);
pref("embedlite.azpc.json.longtap", true);
pref("embedlite.azpc.json.scroll", false);

For some reason I must have assumed that the value in EmbedLiteViewChild.cpp would take precedence, where as in fact it's the other way around. There's another commit that relates to this which is also worth taking note of. This is the commit that lives in the Sailfish part of the gecko code, which has control over the embedding.js file. Here's the commit description:

$ git log -1 -S "embedlite.azpc.json.doubletap" embedding/embedlite/
    embedding.js
commit 720b4f6574e76abaec66712f8ec6aec59bc5dd1f
Author: David Llewellyn-Jones <david@flypig.co.uk>
Date:   Sat Dec 2 22:54:23 2023 +0000

    Convert AddBoolVarCache variables to static prefs
    
    The VarChache has been removed so all instances of AddBoolVarCache() had
    been removed and turned into fixed boolean values. These changes turn
    them into static prefs so that they are configurable again. Use of
    static preferences ensures that we can still have efficient access.

And here's the change, which highlights how the values were removed from the embedding.js file as well, due to them being moved into StaticPrefList.yaml:

$ git diff 720b4f6574e76a~ 720b4f6574e76a -- embedding/embedlite/embedding.js
diff --git a/embedding/embedlite/embedding.js b/embedding/embedlite/embedding.js
index 7853edece218..8156a1664be3 100644
--- a/embedding/embedlite/embedding.js
+++ b/embedding/embedlite/embedding.js
@@ -108,23 +108,6 @@ pref("media.gstreamer.enable-blacklist", false);
 // Disable X backend on GTK
 pref("gfx.xrender.enabled", false);
 
-// AZPC overrides, see EmbedLiteViewChild.cpp
-pref("embedlite.azpc.handle.viewport", true);
-pref("embedlite.azpc.handle.singletap", false);
-pref("embedlite.azpc.handle.longtap", false);
-pref("embedlite.azpc.handle.scroll", true);
-pref("embedlite.azpc.json.viewport", true);
-pref("embedlite.azpc.json.singletap", true);
-pref("embedlite.azpc.json.doubletap", true);
-pref("embedlite.azpc.json.longtap", true);
-pref("embedlite.azpc.json.scroll", false);
-
-// Make gecko compositor use GL context/surface provided by the application.
-pref("embedlite.compositor.external_gl_context", false);
-// Request the application to create GLContext for the compositor as
-// soon as the top level PuppetWidget is creted for the view. Setting
-// this pref only makes sense when using external compositor gl context.
-pref("embedlite.compositor.request_external_gl_context_early", 
    false);
 pref("extensions.update.enabled", false);
 pref("extensions.systemAddon.update.enabled", false);

So there we have it: mystery solved. There were two conflicting default settings in ESR 78 and I somehow managed to pick the wrong one. But with the correct value set, double-tap is now working correctly.

That means that all four of the functionalities that were broken on the Jolla test pages are now fixed and I can tick off this item in the list of browser functionality tests.

The list is looking pretty solid now. The two remaining tasks I need to tackle are audio and video decoding. I have patches from ESR 78 which, if they apply cleanly, might fix these really easily. But if the patches don't apply, these could be really challenging to fix because I really no very little about the area. But that will be something for me to worry about tomorrow!

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.