flypig.co.uk

This morning the packages had completed their build so I'm currently transferring them over to my device ready to test. Yesterday I changed a single preference, the following:

security.external_protocol_requires_permission

Setting its default value in all.js from true to false. This morning I noticed another change I should have made. The lazy getter for this preference also has a default value:

XPCOMUtils.defineLazyPreferenceGetter(
  nsContentDispatchChooser,
  "isPermissionEnabled",
  "security.external_protocol_requires_permission",
  true
);

I should have set this to false as well. Luckily this is in the JavaScript code so I can safely make this change on-device before I do my testing.

The sharp-eyed amongst you may also have noticed that all.js is a JavaScript file as well, so why did I have to do a full rebuild of the packages for that one? That's a really good question with a fascinating answer.

For although all.js has a JavaScript extensions, and although the contents looks just like JavaScript, it is in fact not. We can see this by visiting the comments at the top of the file where we find the following:

// For the syntax used by this file, consult the comments at the top of
// modules/libpref/parser/src/lib.rs.

Okay, let's take the advice and check out the comments at the top of the libpref library source.
 

The description of this syntax continues a bit further; I've cut it for brevity, but what I've included here gives enough of an idea. This highlights that the file has its own grammar that's separate from the JavaScript grammar, and while it's very similar to JavaScript, it's not exactly the same. For example a preference line in all.js is required to finish with a semicolon, whereas this is optional in JavaScript. Mozilla's documentation confirms the format is in fact different:
 
This is why I couldn't just change the value on the device for testing this. But that's a bit of an aside and during the time I've had to write about it the packages have copied over to my phone.

I've now installed the new packages, edited the ContentDispatchChooser.jsm file and packaged it up into omni.ja. I've also removed the preference from the prefs.js in the profile. Time to fire up the browser and test it out...

...And it works! Great, that's another thing sorted; time to move on to the next one:

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

We've been through the first two of these and they're both fixed. Time to move to the third: fullscreen. Fullscreen is useful in a variety of situations, such as when playing videos. The easiest way to test it is using Jolla's test pages. There are five pages related to fullscreen, but I'll probably focus on two: the short page and the long page each containing a fullscreen toggle.

Currently when I press on the "Go to fullscreen" button nothing happens. Neither does the browser go to fullscreen, nor is there any error output to the console. So right now it's all a bit mysterious. Time to investigate.

The first place to find out more is in the JavaScript of the page itself. There we find this function used for toggling the fullscreen mode:

function toggleFullScreen() {
    if (!listenerRegistered) {
        document.onmozfullscreenchange = updateState;
        listenerRegistered = true;
    }

    if (!document.mozFullScreenElement) {
        document.documentElement.mozRequestFullScreen();
    } else {
        if (document.mozFullScreen) {
            document.mozCancelFullScreen();
        }
    }
}

There seem to be three key document related attributes here:

1. The onmozfullscreenchange property for determining the current state.
2. The mozRequestFullScreen() method for entering fullscreen mode.
3. The mozCancelFullScreen() method for exiting fullscreen mode.

That means we have somewhere to start by searching the code for some of these strings. The first place to search should always be the patches and that immediately throws up something that looks relevant: patch 0054 "Make fullscreen enabling work as used to with pref full-screen-api.content-only"

The patch describes itself like this:
 
Okay, that makes sense. The patch itself is really short; all it does is if-def out a small amount of code in nsGlobalWindowOuter.cpp:

+  // We don't have chrome from doc shell point of view. This commit
+  // sha1 3116f3bf53df offends fullscreen API to work without chrome
+  // and shall not make root docShell act as chrome. We previously had
+  // "full-screen-api.content-only" pref set to "true".
+#if 0
   // make sure we don't try to set full screen on a non-chrome window,
   // which might happen in embedding world
   if (mDocShell->ItemType() != nsIDocShellTreeItem::typeChrome)
     return NS_ERROR_FAILURE;
+#endif

Checking the current source code in ESR 91 I can see I've not yet applied this patch. So the obvious thing to do at this point would be to apply it, rebuild and test. On visual inspection I can't see any reason why the patch shouldn't apply cleanly.

And it couldn't have been cleaner:

$ git am --3way \
    ../rpm/0054-sailfishos-gecko-Make-fullscreen-enabling-work-as-us.patch
Applying: Make fullscreen enabling work as used to with pref 
    full-screen-api.content-only. Fixes JB#44129

I've kicked off a build, so now it's just a case of waiting until the build completes. In retrospect I could have run a partial build, but it's just about to be the start of my work day, which means an eight hour stretch when I can happily leave my laptop building. It should be complete by the time I return to it this evening.

[...]

The build completed and — great news — fullscreen now works correctly! It's funny how hard it is to tell in advance whether an issue is going to be easy or hard to fix. The apparently simplest thing can turn out to be a convoluted journey, even if the solution is simple. Something that looks equally challenging if not more so can turn out to be solved by just applying the existing patch. I realise I'm benefiting from someone else's hard work (in this case Raine's) when I do this, but it's a pleasant surprise for it to work so seamlessly.

Alright, that's the third of our four-item issue list completed. Next up is double-tap. If I recall correctly, the double-tap issue relates to double-tap-to-zoom. Zooming is working correctly, but for items that support double-tap we want the double-tap action to take precedence over double-tap-to-zoom. Currently it's the other way around, so this will need fixing.

That'll be my task for tomorrow.

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