flypig.co.uk

We're finally reaching the end of the "Print web page to PDF" saga. Yesterday I re-implemented the JavaScript changes that were in gecko-dev as part of the embedlite-components package. Today I'm going to commit all of the changes so that they're available in the remote repositories as well as my local ones.

Going over the changes I previously made to the gecko-dev code to support the functionality I'm happy to see that all of them are now unnecessary. That means I can completely remove the last two commits.

$ git checkout -b temp-FIREFOX_ESR_91_9_X_RELBRANCH_patches
$ git checkout FIREFOX_ESR_91_9_X_RELBRANCH_patches
$ git log -5 --oneline

2fb912372c04 (HEAD -> FIREFOX_ESR_91_9_X_RELBRANCH_patches,
    temp-FIREFOX_ESR_91_9_X_RELBRANCH_patches)
    Call Print from the CanonicalBrowingContext
26259399358f Reintroduce PDF printing code
e035d6ff3a78 (origin/FIREFOX_ESR_91_9_X_RELBRANCH_patches)
    Add embedlite static prefs
5ae7644719f8 Allow file scheme when loading OpenSearch providers
1fe7c56c7363 Supress URLQueryStrippingListService.jsm error

$ git reset --hard HEAD~~
HEAD is now at e035d6ff3a78 Add embedlite static prefs
$ git log -5 --oneline

e035d6ff3a78 (HEAD -> FIREFOX_ESR_91_9_X_RELBRANCH_patches,
    origin/FIREFOX_ESR_91_9_X_RELBRANCH_patches)
    Add embedlite static prefs
5ae7644719f8 Allow file scheme when loading OpenSearch providers
1fe7c56c7363 Supress URLQueryStrippingListService.jsm error
80fa227aee92 [sailfishos][gecko] Fix gfxPlatform::AsyncPanZoomEnabled
    for embedlite. JB#50863
b58093d23fb2 Add patch to fix 32-bit builds

These commits would have had to be turned into one or maybe two patches to the upstream code, so the fact I can remove them completely leaves me with a warm feeling inside.

Almost all of these changes are to the JavaScript code, so wouldn't require a rebuild of gecko-dev to apply them on my device in themselves. However there is one change to the nsIWebBrowserPrint interface which will have a knock on effect to the compiled (C++ and Rust) code as well, which is this one:

@@ -98,7 +98,7 @@ interface nsIWebBrowserPrint : nsISupports
    * @param aWPListener - is updated during the print
    * @return void
    */
-  [noscript] void print(in nsIPrintSettings aThePrintSettings,
                         in nsIWebProgressListener aWPListener);
+  void print(in nsIPrintSettings aThePrintSettings,
 
   /**

Since we've shifted from calling print() on the window to calling print() on the canonical browsing context, this change shouldn't be needed any more. But to check that this really is the case I'm going to have to do a full rebuild. Having reverted the changes above I can therefore kick things off:

$ sfdk -c no-fix-version build -d -p --with git_workaround

While this is building I've also created the issue that we considered yesterday as well. It's now Issue #1051: "Fix hang when calling window.setBrowserCover()".

I also created another issue that I've been noticing during testing of the ESR 91 code. There are various Web pages which don't work as well as expected, meaning that they don't work as well on ESR 91 as they do on ESR 78. That's not good for the end user. Previous versions of the browser have had similar problems and invariably the issue relates to what the site is serving to the browser. If the server doesn't recognise the User Agent string it will often send a different version of the page. Often these versions are broken or at least don't work well with the gecko rendering engine. It's pretty astonishing that this is still necessary in a world where Web standards are so much more clearly defined than they were even a decade ago, but there it is.

I've not yet looked into it but I have a suspicion that the reason some of these sites may not be working is that the Sailfish Browser's user agent string handling code is no longer sending the "adjusted" user agent strings to the sites that need them. So I've created Issue 1052 for looking in to this and trying to fix it if there's something to fix.

The gecko-dev build has completed (it took most of the day, but at least not all of the day). I'm now rebuilding the packages that depend on it so that I can then test them out.

It's almost the full suite: gecko-dev, qtmozembed-qt5, embedlite-components and sailfish-browser. The only package I've not rebuilt is sailfish-components-webview. I've not yet made any changes to that at all since starting this ESR 91 work.

All built. All uploaded.

$ devel-su rpm -U --force xulrunner-qt5-91.*.rpm \
    xulrunner-qt5-debugsource-91.*.rpm xulrunner-qt5-debuginfo-91.*.rpm \
    xulrunner-qt5-misc-91.*.rpm qtmozembed-qt5-1.*.rpm \
    qtmozembed-qt5-debuginfo-1.*.rpm qtmozembed-qt5-debugsource-1.*.rpm \
    sailfish-browser-2.*.rpm sailfish-browser-debuginfo-2.*.rpm \
    sailfish-browser-debugsource-2.*.rpm sailfish-browser-settings-2.*.rpm \
    embedlite-components-qt5-1.*.rpm \
    embedlite-components-qt5-debuginfo-1.*.rpm \
    embedlite-components-qt5-debugsource-1.*.rpm

All installed.

Now to test the changes... and it doesn't work. That's no fun.

But as soon as I look at the code it's clear what the problem is. Here's the error:

[JavaScript Error: "aSerializable.url is undefined"
    {file: "resource://gre/modules/DownloadCore.jsm" line: 1496}]
DownloadSource.fromSerializable@resource://gre/modules/DownloadCore.jsm:1496:5
Download.fromSerializable@resource://gre/modules/DownloadCore.jsm:1282:38
D_createDownload@resource://gre/modules/Downloads.jsm:108:39
DownloadPDFSaver2.fromSerializable@file:///usr/lib64/mozembedlite/components/
    EmbedliteDownloadManager.js:420:34
observe/<@file:///usr/lib64/mozembedlite/components/
    EmbedliteDownloadManager.js:272:56

Apart from the error in DownloadCore.jsm I'm also a little perturbed at the reference to DownloadPDFSaver2. While I had two versions of DownloadPDFSaver in the code at the same time I did call one of them DownloadPDFSaver2, but subsequently switched all references to just use DownloadPDFSaver (or so I thought) once I'd removed the unused version. So that really shouldn't be appearing at all.

Let's find out why.

First the reference to DownloadPDFSaver2. When I check the actual code on device it certainly does still reference this, but that's different to the code I have on my laptop. I must have made a mistake when building the package. I've rebuilt it, re-uploaded it and reinstalled it. Now when I run it I get this:

[JavaScript Error: "aSerializable.url is undefined"
    {file: "resource://gre/modules/DownloadCore.jsm" line: 1496}]
DownloadSource.fromSerializable@resource://gre/modules/DownloadCore.jsm:1496:5
Download.fromSerializable@resource://gre/modules/DownloadCore.jsm:1282:38
D_createDownload@resource://gre/modules/Downloads.jsm:108:39
DownloadPDFSaver.createDownload@file:///usr/lib64/mozembedlite/components/
    EmbedliteDownloadManager.js:426:34
observe/<@file:///usr/lib64/mozembedlite/components/
    EmbedliteDownloadManager.js:272:55

That looks more healthy. Now for the main error. It looks to me like this is due to the removal of the following from DownloadCore.jsm:

@@ -1499,12 +1491,6 @@ DownloadSource.fromSerializable = function(aSerializable) {
     source.url = aSerializable.toString();
   } else if (aSerializable instanceof Ci.nsIURI) {
     source.url = aSerializable.spec;
-  } else if (aSerializable instanceof Ci.nsIDOMWindow) {
-    source.url = aSerializable.location.href;
-    source.isPrivate = PrivateBrowsingUtils.isContentWindowPrivate(
-      aSerializable
-    );
-    source.windowRef = Cu.getWeakReference(aSerializable);
   } else {
     // Convert String objects to primitive strings at this point.
     source.url = aSerializable.url.toString();

In the new code in EmbedliteDownloadManager.js we send in the source like this:

  let download = await DownloadPDFSaver.createDownload({
    source: Services.ww.activeWindow,
    target: data.to
  });

But now that I removed the code from DownloadCore.jsm it no longer knows how to handle this Services.ww.activeWindow value. I should be handling that in EmbedliteDownloadManager.js instead. At present the code that passes this value on looks like this:

DownloadPDFSaver.createDownload = async function(aProperties) {
  let download = await Downloads.createDownload({
    source: aProperties.source,
    target: aProperties.target,
    contentType: "application/pdf"
  });

So I should be able to fix this by handling it more judiciously. Let's try this:

DownloadPDFSaver.createDownload = async function(aProperties) {
  let download = await Downloads.createDownload({
    source: aProperties.source.location.href,
    target: aProperties.target,
    contentType: "application/pdf"
  });
  download.source.isPrivate = PrivateBrowsingUtils.isContentWindowPrivate(
    aProperties.source
  );
  download.source.windowRef = Cu.getWeakReference(aProperties.source);

This way the deserialisable in DownloadCore will just accept the source value as a string and store it in source.url (just as we need). We can then embellish it with the additional isPrivate and windowRef elements as in the code above.

In order to get this to work I also have to pull in PrivateBrowsingUtils at the top of the file:

const { PrivateBrowsingUtils } = ChromeUtils.import(
  "resource://gre/modules/PrivateBrowsingUtils.jsm"
);

The end result is working. I've pushed all the changes to the remote repositories. Phew. Finally I'm going to call this task done.

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