List items
Items from the current list are shown below.
Blog
All items from September 2024
15 Sep 2024 : Week 49 Summary #
It's been a week since I last posted a diary entry. It turns out that not writing a diary entry everyday is almost as hard as writing one. If I'm not wandering aimlessly around the house trying to find a purpose, I find I'm sitting on the sofa quietly mumbling gdb backtrace nonsense to myself.
Thankfully there are still some tasks to do, addressing review comments and reviewing others' commits. I'll talk about these happenings today. But I also want to share some retrospective analysis of the project: what went well, what went badly and how the whole process felt as an experience. I'll be expanding on all that in a future post.
First up I want to highlight the really nice fix from from Vlad G (vlagged) that's fixed certificate overrides. When the browser accesses a site using HTTPS but receives a bad certificate, it'll present the user with an error page. This is to avoid the user mistakenly believing that the data being sent from the site, or that they're sending back to it, is being done so securely (i.e. using a channel that maintains integrity and confidentiality).
There are still some sites that don't support HTTPS which provide no more security than a site with a broken or bad certificate. But the latter is worse in that could give the user a false sense of security. If there's a failure it could be an attack. Exposing this to the user in an overt way is critical for them to make an informed decision about how to proceed.
That means that Firefox typically offers the option to view the site anyway, as long as the warning page has been prominently presented. It's hard to imagine anyone could miss the bad certificate page given the way it's presented on Sailfish OS.
Changes to the underlying API however mean that the ESR 91 upgrade broke the option to bypass the certificate check. With this broken code, selecting the option to "view the page anyway" caused the browser to return the same certificate warning page rather than actually going to the site. That's because the button is supposed to record a "certificate override" stating that the browser should ignore the bad certificate. But the method to perform this has changed parameters, so it was throwing up an error rather than recording the site details.
Here we can see the blame for the interface file where the new parameter was added by R. Martinho Fernandes:
As I write this I've approved the change myself and hopefully it'll be picked up and reviewed by Jolla so that it can be merged swiftly in to the ESR 91 branch.
Besides this I also received really nice review feedback from sailors Raine (rainemak) and Mal (mlehtima) in relation to my gecko-dev pull request. This pull request is the one that actually performs the upgrade from ESR 78 to ESR 91. The upgrade takes the form of redirecting a submodule to a different upstream commit so is a super-simple change. The bigger part is the set of patches that then has to be applied on top of the upstream code to make it all work. This is all in the pull request as well.
Raine and Mal clearly spent some time going through the commits and I'm really grateful for their suggestions. The result was better preference handling and several commits being merged or removed entirely. On Mal's request I also rebased the code to bring in recent changes needed to support Jolla's updated SDK. The totality of these changes all make for a much cleaner commit history and impoved code.
This pull request, along with the pull requests for the sailfish-browser and sailfish-components-webview repositories, are now awaiting a second review from the Jolla team. As far as I'm aware I've completed the changes requested, but there may still be a few more review rounds to go before they get merged in.
The other change this week has been to the package tarballs available from my website. These are very much still testing packages and you should read all of the info in the installation instructions before you use them. Nevertheless these new packages should provide some nice improvements over the previous ones. The main changes are:
You should, however, still do step 5 to clean out the old settings files. This won't clear cookies, tabs, history or anything like that, but it will reset any preferences you've changed using about:config. In summary, if you already have ESR 91 installed you should complete the following steps:
That summarises the changes this week. I'm still writing up my retrospective of the project so far and will post it once it's ready.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Comment
Thankfully there are still some tasks to do, addressing review comments and reviewing others' commits. I'll talk about these happenings today. But I also want to share some retrospective analysis of the project: what went well, what went badly and how the whole process felt as an experience. I'll be expanding on all that in a future post.
First up I want to highlight the really nice fix from from Vlad G (vlagged) that's fixed certificate overrides. When the browser accesses a site using HTTPS but receives a bad certificate, it'll present the user with an error page. This is to avoid the user mistakenly believing that the data being sent from the site, or that they're sending back to it, is being done so securely (i.e. using a channel that maintains integrity and confidentiality).
There are still some sites that don't support HTTPS which provide no more security than a site with a broken or bad certificate. But the latter is worse in that could give the user a false sense of security. If there's a failure it could be an attack. Exposing this to the user in an overt way is critical for them to make an informed decision about how to proceed.
That means that Firefox typically offers the option to view the site anyway, as long as the warning page has been prominently presented. It's hard to imagine anyone could miss the bad certificate page given the way it's presented on Sailfish OS.
Changes to the underlying API however mean that the ESR 91 upgrade broke the option to bypass the certificate check. With this broken code, selecting the option to "view the page anyway" caused the browser to return the same certificate warning page rather than actually going to the site. That's because the button is supposed to record a "certificate override" stating that the browser should ignore the bad certificate. But the method to perform this has changed parameters, so it was throwing up an error rather than recording the site details.
Here we can see the blame for the interface file where the new parameter was added by R. Martinho Fernandes:
$ git blame -c --date=short \
security/manager/ssl/nsICertOverrideService.idl -L 84,113
[...]
345598abc1d6f (Masatoshi Kimura 2019-06-11) void rememberValidityOverride(
in AUTF8String aHostName,
e368dc9c853cf (Ehsan Akhgari 2012-08-22) in int32_t aPort,
7ea1bcd018a32 (R. Martinho Fernandes 2021-06-01) in jsval aOriginAttributes,
61d16d4cd9588 (kaie@kuix.de 2007-10-03) in nsIX509Cert aCert,
e368dc9c853cf (Ehsan Akhgari 2012-08-22) in uint32_t aOverrideBits,
f2e2a270fb9a4 (kaie@kuix.de 2007-11-19) in boolean aTemporary);
If we check out the commit 7ea1bcd018a32 that introduced the change, we can see it takes us to differential revision D91962:
$ git log -1 7ea1bcd018a32
commit 7ea1bcd018a32a8e98216cf9f675c49d582950ca
Author: R. Martinho Fernandes <bugs@rmf.io>
Date: Tue Jun 1 06:55:07 2021 +0000
Bug 1597600 - make certificate overrides depend on origin attributes
r=keeler,geckoview-reviewers,smaug,agi
Differential Revision: https://phabricator.services.mozilla.com/D91962
This was noticed by Vlad who dug down into the code and found the source of the error, fixing it by adding the missing parameter. It's great detective work and a nice fix.As I write this I've approved the change myself and hopefully it'll be picked up and reviewed by Jolla so that it can be merged swiftly in to the ESR 91 branch.
Besides this I also received really nice review feedback from sailors Raine (rainemak) and Mal (mlehtima) in relation to my gecko-dev pull request. This pull request is the one that actually performs the upgrade from ESR 78 to ESR 91. The upgrade takes the form of redirecting a submodule to a different upstream commit so is a super-simple change. The bigger part is the set of patches that then has to be applied on top of the upstream code to make it all work. This is all in the pull request as well.
Raine and Mal clearly spent some time going through the commits and I'm really grateful for their suggestions. The result was better preference handling and several commits being merged or removed entirely. On Mal's request I also rebased the code to bring in recent changes needed to support Jolla's updated SDK. The totality of these changes all make for a much cleaner commit history and impoved code.
This pull request, along with the pull requests for the sailfish-browser and sailfish-components-webview repositories, are now awaiting a second review from the Jolla team. As far as I'm aware I've completed the changes requested, but there may still be a few more review rounds to go before they get merged in.
The other change this week has been to the package tarballs available from my website. These are very much still testing packages and you should read all of the info in the installation instructions before you use them. Nevertheless these new packages should provide some nice improvements over the previous ones. The main changes are:
- The user agent patch has now been applied, so problems with pages serving desktop rather than mobile variants should now be largely resolved.
- Vlad's HTTPS fix is included so you can now view pages with broken TLS certificates if you so choose.
You should, however, still do step 5 to clean out the old settings files. This won't clear cookies, tabs, history or anything like that, but it will reset any preferences you've changed using about:config. In summary, if you already have ESR 91 installed you should complete the following steps:
- Download the tarball to your phone.
$ curl -O https://www.flypig.co.uk/dnload/dnload/sailfishos/gecko/gecko-dev-esr91-release.tar.bz2
- Unpack the tarball:
$ tar -xvf gecko-dev-esr91-release.tar.bz2
- Ensure the browser and any apps using the WebView are closed.
- [Skip this step]
- Remove settings that need to be updated for ESR 91:
$ rm ~/.local/share/org.sailfishos/browser/.mozilla/ua-update.json $ rm -rf ~/.local/share/org.sailfishos/browser/.mozilla/startupCache $ rm ~/.local/share/org.sailfishos/browser/.mozilla/prefs.js $ rm ~/.local/share/org.sailfishos/browser/__PREFS_WRITTEN__
- Install the ESR 91 packages:
$ devel-su rpm -U --force \ xulrunner-qt5-91.*.rpm \ xulrunner-qt5-misc-91.*.rpm \ qtmozembed-qt5-1.*.rpm \ sailfish-components-webview-qt5-1.*.rpm \ sailfish-components-webview-qt5-pickers-1.*.rpm \ sailfish-components-webview-qt5-popups-1.*.rpm \ embedlite-components-qt5-1.*.rpm \ sailfish-browser-2.*.rpm \ sailfish-browser-settings-2.*.rpm \ mapplauncherd-booster-browser-0.*.rpm - [Skip this step]
- Finally kill the browser booster:
$ killall booster-browser
- You're good to go!
That summarises the changes this week. I'm still writing up my retrospective of the project so far and will post it once it's ready.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
8 Sep 2024 : Day 339 #
It's been just under a week since I posted my last diary entry. In the meantime I've been attending RSECon24 learning about assorted software engineering topics and meeting others in the research software engineering community. But you'll be pleased to hear that I did also find some time for gecko dev work as well.
Not as much as I would normally do, so in retrospect pausing my diary entries was the right thing to do. But it's been enough progress to justify my promised summary today.
Before we get into the details of what I've personally been working on I want to first talk about what others have been doing, as well as considering the future of these dev diaries.
Before heading to the conference I submitted five pull requests:
And that seems to have been borne out by how things have been moving since then. There have been exciting developments pulling in input from across the Sailfish community and beyond, from both developers and users. I'd like to highlight just some of the things that have started happening recently and which give me great optimism for the future of both Sailfish OS browser development and the Sailfish OS platform more broadly.
Started by Frajo (krnlyng), but then contributed to by Mal (mlehtima) and Affe Nul (affenull2345), the hangs and crashes coming from the EGL rendering pipeline have been carefully ironed out in the libhybris layer. The solutions were completely unexpected as far as I was concerned and I'd never have been able to figure them out myself, so this was a source of much celebration and admiration from me.
There have also been other pull requests to address actual issues in the browser brought on by the upgrade. There are so many features in a browser it's very hard to test everything (an argument for good unit tests, but that's a real challenge given the discrete approach to upgrades required for Gecko on Sailfish OS). A great example of this is the pull request submitted by Vlad G (vlagged) to fix bad certificate overrides. It was easy for me to miss this functionality. Vlad has been a dedicated follower of the dev diaries and it's important not to underestimate how important contributions like this are. It's this kind of diligent, proactive and practical help that the browser really need to be a success.
Quite apart from the motivation it's given me over the last year, the discussion on the Sailfish forum has driven some important advances for the browser. There's been a healthy discussion about sites that have glitches or aren't working correctly, for example from Tone (tortoisedoc), Daniel (jauri.gagarin.II), Sebastian (smatkovi) and others.
I'm hoping that many of these issues will turn out to be related to user-agent strings and the fact that I'd missed off an important user-agent patch. This was flagged up through a discussion between Attah (attah) and Raine (rainemak). Now that the patch has been applied, thanks to their work, many of the pages will hopefully work more successfully.
Members of the Sailfish community have invested their time and energy into testing ESR 91 on a range of difference devices, including Pasi (Pasik2) testing on the Pinetab 2, Ladislav Hodas (lhodas) on the Gemini and Affe Nul (affenull2345) on the Samsung Galaxy A40. Matti Viljanen (direc85) deserves a special mention for getting complete rebuilds working for armv7hl and i486 versions of the packages, demonstrating that this can be achieved with minimal changes. From what I understand there are still issues to be worked out with the resulting packages, but this is work that I'd have struggled to do on my own.
Sticking with Matti, who I'm very pleased to see is now working for Jolla (congratulations!), he provided me with excellent advice on improving the build both in public and private communications, especially in relation to the Rust components. On the topic of Rust, I also received really useful suggestions from Tone (tortoisedoc) which also has the potential to improve Rust builds.
There was some useful discussion on the forum about the WebRender pipeline and its relationship to Progressive Web Apps, with Simon Schmeisser (simonschmeisser), Tone (tortoisedoc), Attah (attah) and olf (Olf0) all contributing. It was a worthwhile read covering important ground. These are the sorts of topics that will deserve even more discussion once we start thinking about ESR 102 (it will happen!).
I can't possibly mention everyone who's spent time encouraging me over the last year, but let me just say that it was gratifying to receive such kind remarks on the forum from so many users who tried out the packages, including Harry (hschmitt), hanswf, Helge, Patrick (pherjung), ric9k, 247, Tuomas Nurmi (Mazoon), Christian (p1rat), Maximilian (Maximilian1st), Edz, Uli (Fellfrosch), Mark Washeim (poetaster), Throwaway69, eson, Gabriele (gabrigubin), Felix (FelixWilke), Ricardo (monkeyisland) and KeksKopf (davidrasch), as well as many more on Mastodon.
And finally, the really hard work will now fall to the Jolla team. Raine (rainemak) and Mal (mlehtima) have already provided invaluable review comments. Alongside Frajo (krnlyng), Matti (direc85) and no doubt others, I'm hoping they'll be able to pick up some of this work to chisel off the sharper corners and sand down the rough edges. The result of Jolla's unparalleled engineering expertise will give a better result than I could ever hope to achieve.
An important thread that runs throughout these contributions is that I'd have struggled to do all this on my own. Focused on the code, I've not been spending my time trying to persuade others to contribute. Members of the community have therefore taken their own initiative to help and the browser has benefited enormously as a result.
This list turned out far longer than I was expecting and by necessity captures only some of the exciting developments that have been driven by the community recently. I'm very excited to see how things continue to progress.
But it also highlights the challenge that's now presented to me as I write this development diary. The work has become more distributed and this will inevitably dilute my contributions in a positive way. That's a great thing and my expectation is that I'll likely now spend less of my time directly working on the code and more of my time on the processes needed to get the browser released.
That means a daily development diary probably doesn't make much sense any more. Up until now the act of writing daily has been critical in driving me forwards, ensuring I don't lose focus and keeping me on track. That's no longer needed; the browser now has its own momentum.
But I do still want to maintain a record of progress. I hope to capture developments up until the browser is properly released, assuming it can get to that point. So my plan for this diary in the future is to write a weekly update on how things are progressing. If there's not much to write about, I'll write up my thoughts on a topic related to Gecko and Sailfish OS more generally. I'll publish them at the weekend because that's when the other pieces of my life are least frantic, giving me the opportunity to consolidate my thoughts from the week gone by. To reflect this change in pace I'll no longer count the days but rather give each post a proper title.
That's my plan. If it doesn't work out, I can always change it. Please do share your thoughts about it with me as I'm always grateful to receive them.
With all this in mind, let's return to those pull requests and talk about what's happened to them over the last week. I'm really happy that two of them have already been merged into the upstream repository. They're not on the main branch yet, which is still on ESR 78, but this is an important step towards that.
That leaves three remaining. Raine and Mal both added comments to these which left me with a bit of work to do to address their requests. Some of these changes were straightforward, like removing extraneous empty lines that I added during development and forgot to remove.
Changes like these have no impact on the code semantics and so wouldn't even necessarily justify a rebuild. But they still take a surprising length of time to address. If they're part of the mirrored gecko code the repository has to be reset, cleaned and all patches applied. Once the change has been made it needs to be squashed into the commit where it was added. Then the patches need to be regenerated. Only then can the changes be committed and pushed to the remote repository.
This is one of the reasons I'm not keen on working with patches. I find myself repeatedly having to clean the repository, apply the patches and then regenerate them before a fresh set of commits can be pushed. I find it surprisingly laborious. And while I fully understand the reasons for using them (the rpm build process has been tailored for this approach) that doesn't make me like them any more.
That's for the simplest of changes, but some of the changes are more substantial. For example Raine requested various changes to how preferences should be applied, all of which are very legitimate.
These changes centre around Sailfish OS specific preferences which differ depending on whether the browser or WebView is in use.
In these cases they can either be set to the default of the browser or set to the default of the WebView. Whichever way is chosen, some code is needed in either the WebView or the browser to ensure the value is correctly toggled from the default for the other.
My original code set the value to a default that worked for the browser, then flipped them in the WebView initialisation code. From what I understand, Raine was keen for me to do the opposite. That is, to set defaults that work for the WebView but are then flipped by the browser initialisation code. The request itself was spread over several or Raine's comments, but the following is perhaps the pithiest of the descriptions:
Consequently I updated the default values set in the gecko code, removed some code from the WebView and added some equivalent code to the browser like this:
Having made these changes I then merged them in to the existing branch. To keep things clean I've squashed the changes into relevant commits. But this did also mean spending a bit of time rebasing all of the commits in the pull request.
The resulting in changes to all three of the outstanding pull requests:
Consequently I've pushed the changes upstream to the pull requests. My timing isn't ideal, right before the weekend, but I'll look forward to hearing more from the Jolla team in due course.
That's it for today. I'll return next week with a summary of changes that have taken place during the week. I'm also planning a retrospective to summarise the entire process of the last 339 days of development: what went well, what could have gone better and what the experience has been like for me.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Comment
Not as much as I would normally do, so in retrospect pausing my diary entries was the right thing to do. But it's been enough progress to justify my promised summary today.
Before we get into the details of what I've personally been working on I want to first talk about what others have been doing, as well as considering the future of these dev diaries.
Before heading to the conference I submitted five pull requests:
- https://github.com/sailfishos/gecko-dev/pull/162
- https://github.com/sailfishos/qtmozembed/pull/49
- https://github.com/sailfishos/sailfish-browser/pull/1074
- https://github.com/sailfishos/embedlite-components/pull/100
- https://github.com/sailfishos/sailfish-components-webview/pull/169
And that seems to have been borne out by how things have been moving since then. There have been exciting developments pulling in input from across the Sailfish community and beyond, from both developers and users. I'd like to highlight just some of the things that have started happening recently and which give me great optimism for the future of both Sailfish OS browser development and the Sailfish OS platform more broadly.
Started by Frajo (krnlyng), but then contributed to by Mal (mlehtima) and Affe Nul (affenull2345), the hangs and crashes coming from the EGL rendering pipeline have been carefully ironed out in the libhybris layer. The solutions were completely unexpected as far as I was concerned and I'd never have been able to figure them out myself, so this was a source of much celebration and admiration from me.
There have also been other pull requests to address actual issues in the browser brought on by the upgrade. There are so many features in a browser it's very hard to test everything (an argument for good unit tests, but that's a real challenge given the discrete approach to upgrades required for Gecko on Sailfish OS). A great example of this is the pull request submitted by Vlad G (vlagged) to fix bad certificate overrides. It was easy for me to miss this functionality. Vlad has been a dedicated follower of the dev diaries and it's important not to underestimate how important contributions like this are. It's this kind of diligent, proactive and practical help that the browser really need to be a success.
Quite apart from the motivation it's given me over the last year, the discussion on the Sailfish forum has driven some important advances for the browser. There's been a healthy discussion about sites that have glitches or aren't working correctly, for example from Tone (tortoisedoc), Daniel (jauri.gagarin.II), Sebastian (smatkovi) and others.
I'm hoping that many of these issues will turn out to be related to user-agent strings and the fact that I'd missed off an important user-agent patch. This was flagged up through a discussion between Attah (attah) and Raine (rainemak). Now that the patch has been applied, thanks to their work, many of the pages will hopefully work more successfully.
Members of the Sailfish community have invested their time and energy into testing ESR 91 on a range of difference devices, including Pasi (Pasik2) testing on the Pinetab 2, Ladislav Hodas (lhodas) on the Gemini and Affe Nul (affenull2345) on the Samsung Galaxy A40. Matti Viljanen (direc85) deserves a special mention for getting complete rebuilds working for armv7hl and i486 versions of the packages, demonstrating that this can be achieved with minimal changes. From what I understand there are still issues to be worked out with the resulting packages, but this is work that I'd have struggled to do on my own.
Sticking with Matti, who I'm very pleased to see is now working for Jolla (congratulations!), he provided me with excellent advice on improving the build both in public and private communications, especially in relation to the Rust components. On the topic of Rust, I also received really useful suggestions from Tone (tortoisedoc) which also has the potential to improve Rust builds.
There was some useful discussion on the forum about the WebRender pipeline and its relationship to Progressive Web Apps, with Simon Schmeisser (simonschmeisser), Tone (tortoisedoc), Attah (attah) and olf (Olf0) all contributing. It was a worthwhile read covering important ground. These are the sorts of topics that will deserve even more discussion once we start thinking about ESR 102 (it will happen!).
I can't possibly mention everyone who's spent time encouraging me over the last year, but let me just say that it was gratifying to receive such kind remarks on the forum from so many users who tried out the packages, including Harry (hschmitt), hanswf, Helge, Patrick (pherjung), ric9k, 247, Tuomas Nurmi (Mazoon), Christian (p1rat), Maximilian (Maximilian1st), Edz, Uli (Fellfrosch), Mark Washeim (poetaster), Throwaway69, eson, Gabriele (gabrigubin), Felix (FelixWilke), Ricardo (monkeyisland) and KeksKopf (davidrasch), as well as many more on Mastodon.
And finally, the really hard work will now fall to the Jolla team. Raine (rainemak) and Mal (mlehtima) have already provided invaluable review comments. Alongside Frajo (krnlyng), Matti (direc85) and no doubt others, I'm hoping they'll be able to pick up some of this work to chisel off the sharper corners and sand down the rough edges. The result of Jolla's unparalleled engineering expertise will give a better result than I could ever hope to achieve.
An important thread that runs throughout these contributions is that I'd have struggled to do all this on my own. Focused on the code, I've not been spending my time trying to persuade others to contribute. Members of the community have therefore taken their own initiative to help and the browser has benefited enormously as a result.
This list turned out far longer than I was expecting and by necessity captures only some of the exciting developments that have been driven by the community recently. I'm very excited to see how things continue to progress.
But it also highlights the challenge that's now presented to me as I write this development diary. The work has become more distributed and this will inevitably dilute my contributions in a positive way. That's a great thing and my expectation is that I'll likely now spend less of my time directly working on the code and more of my time on the processes needed to get the browser released.
That means a daily development diary probably doesn't make much sense any more. Up until now the act of writing daily has been critical in driving me forwards, ensuring I don't lose focus and keeping me on track. That's no longer needed; the browser now has its own momentum.
But I do still want to maintain a record of progress. I hope to capture developments up until the browser is properly released, assuming it can get to that point. So my plan for this diary in the future is to write a weekly update on how things are progressing. If there's not much to write about, I'll write up my thoughts on a topic related to Gecko and Sailfish OS more generally. I'll publish them at the weekend because that's when the other pieces of my life are least frantic, giving me the opportunity to consolidate my thoughts from the week gone by. To reflect this change in pace I'll no longer count the days but rather give each post a proper title.
That's my plan. If it doesn't work out, I can always change it. Please do share your thoughts about it with me as I'm always grateful to receive them.
With all this in mind, let's return to those pull requests and talk about what's happened to them over the last week. I'm really happy that two of them have already been merged into the upstream repository. They're not on the main branch yet, which is still on ESR 78, but this is an important step towards that.
That leaves three remaining. Raine and Mal both added comments to these which left me with a bit of work to do to address their requests. Some of these changes were straightforward, like removing extraneous empty lines that I added during development and forgot to remove.
Changes like these have no impact on the code semantics and so wouldn't even necessarily justify a rebuild. But they still take a surprising length of time to address. If they're part of the mirrored gecko code the repository has to be reset, cleaned and all patches applied. Once the change has been made it needs to be squashed into the commit where it was added. Then the patches need to be regenerated. Only then can the changes be committed and pushed to the remote repository.
This is one of the reasons I'm not keen on working with patches. I find myself repeatedly having to clean the repository, apply the patches and then regenerate them before a fresh set of commits can be pushed. I find it surprisingly laborious. And while I fully understand the reasons for using them (the rpm build process has been tailored for this approach) that doesn't make me like them any more.
That's for the simplest of changes, but some of the changes are more substantial. For example Raine requested various changes to how preferences should be applied, all of which are very legitimate.
These changes centre around Sailfish OS specific preferences which differ depending on whether the browser or WebView is in use.
In these cases they can either be set to the default of the browser or set to the default of the WebView. Whichever way is chosen, some code is needed in either the WebView or the browser to ensure the value is correctly toggled from the default for the other.
My original code set the value to a default that worked for the browser, then flipped them in the WebView initialisation code. From what I understand, Raine was keen for me to do the opposite. That is, to set defaults that work for the WebView but are then flipped by the browser initialisation code. The request itself was spread over several or Raine's comments, but the following is perhaps the pithiest of the descriptions:
I think embedding.js (embedlite omni.ja) should default to false and browser set it true.
Consequently I updated the default values set in the gecko code, removed some code from the WebView and added some equivalent code to the browser like this:
// Ensure the renderer is configured correctly
webEngineSettings->setPreference(QStringLiteral(
"embedlite.compositor.external_gl_context"),
QVariant(true));
webEngineSettings->setPreference(QStringLiteral(
"embedlite.compositor.request_external_gl_context_early"),
QVariant(true));
Thankfully these are also pretty simple changes, but they did need a bit of thought, not least because they touch on three separate packages. The gecko engine, the browser and the WebView code all needed updating, albeit each in only a small way.Having made these changes I then merged them in to the existing branch. To keep things clean I've squashed the changes into relevant commits. But this did also mean spending a bit of time rebasing all of the commits in the pull request.
The resulting in changes to all three of the outstanding pull requests:
- https://github.com/sailfishos/gecko-dev/pull/162
- https://github.com/sailfishos/sailfish-browser/pull/1074
- https://github.com/sailfishos/sailfish-components-webview/pull/169
Consequently I've pushed the changes upstream to the pull requests. My timing isn't ideal, right before the weekend, but I'll look forward to hearing more from the Jolla team in due course.
That's it for today. I'll return next week with a summary of changes that have taken place during the week. I'm also planning a retrospective to summarise the entire process of the last 339 days of development: what went well, what could have gone better and what the experience has been like for me.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
2 Sep 2024 : Day 338 #
As I write this it's the first day of Meteorological Autumn. It still feels very much like summer though (maybe because we won't reach Astronomical Autumn until later in the month). The sun is shining, there's a light breeze, crickets are chirping and there's a generally relaxed atmosphere in the air. But it's the changes in season that are always the most exciting time of year for me. I've enjoyed the summer, but I'm now looking forward to autumn and all of its mellow fruitfulness. Just thinking about it energises me.
But it also represents a shift in gear for this gecko diary as well. I'm hoping to get my pull requests submitted today. That doesn't mean the work is over, but it does mean that it's reached a stage where I feel it can be built upon (by both me and others). Moreover, as I mentioned previously I'll be taking a break this week while I'm attending a conference. So this will be my last diary entry until next weekend.
But that still means there's work to do today and I got up early this morning to check the build I started last night. And...? sadly it didn't complete. Most of the six patches I added last night seem to have built okay, but the startUpCache patch has caused errors. And a lot of errors at that. Although there are pages and pages of errors they all seem to be of one of two flavours.
First there are namespace errors
If I compare the code from ESR 78 it looks pretty similar, but I am a little surprised to see that the newly added Logging.h include is placed inside the namespace qualifiers:
This gives much better results: the majority of the errors are now gone and I'm just left with a couple of errors related to use of NS_LITERAL_STRING() and NS_LITERAL_CSTRING(). This has been a recurrent issue throughout this process since the gecko code now requires use of an _ns string suffix rather than one of the previous NS_LITERAL_STRING() macros.
Having replaced the two string literals with the annotations the code now compiles without error. But it means I have to do a complete build again, which means I have to regenerate the startupCache patch again. The commits have changed because the patch descriptions have changed as a result of being applied by the build process, but other than this I can use the same command to generate the patches as before:
So I've updated the patches, added the changes to the commits, cleaned the entire source tree of cruft and now it's time for another full rebuild.
I want at least one full rebuild to go through without errors before I create this mythical pull request I've been going on about. It's frankly turning out to be a little more laborious to get it to happen than I was expecting. But it'll get there.
[...]
Now finally after a full day of compilation, the final full build has completed. I've copied over and installed the packages and a quick check suggests things are working as expected. The pages are returning correctly as mobile pages. The email app doesn't crash when you switch back from the homescreen. The WebView seems to be working correctly.
The browser does still segfault on exit and, after a discussion with Frajo, it's possible that this could be fixable. But this isn't a new issue and while it'd be great to have it fixed, it could take a while to figure out. So that's going to have to be something for the future.
The task now, finally, is to submit the pull request. In fact, not just one pull request, but all five of them for the various packages that have needed changes needed for supporting the new engine. After a bit of playing around on GitHub, here they are:
I'll return to this next weekend and I'll continue with fresh diary entries then. But right now, I plan to spend a bit of time breathing in and enjoying the fresh autumnal air.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Comment
But it also represents a shift in gear for this gecko diary as well. I'm hoping to get my pull requests submitted today. That doesn't mean the work is over, but it does mean that it's reached a stage where I feel it can be built upon (by both me and others). Moreover, as I mentioned previously I'll be taking a break this week while I'm attending a conference. So this will be my last diary entry until next weekend.
But that still means there's work to do today and I got up early this morning to check the build I started last night. And...? sadly it didn't complete. Most of the six patches I added last night seem to have built okay, but the startUpCache patch has caused errors. And a lot of errors at that. Although there are pages and pages of errors they all seem to be of one of two flavours.
First there are namespace errors
364:14.24 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsILoadInfo.h: In member
function ‘bool nsILoadInfo::
GetAllowListFutureDocumentsCreatedFromThisRedirectChain()’:
364:14.24 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsILoadInfo.h:849:5:
error: reference to ‘mozilla’ is ambiguous
364:14.24 849 | mozilla::DebugOnly<nsresult> rv =
364:14.24 849 | mozilla::DebugOnly rv = GetAllowListFutureDocumentsCreatedFromThisRedirectChain(&result);
364:14.24 | ^~~~~~~
Next there are undefined type errors (in this case nsresult):
364:14.30 ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsILoadInfo.h:962:32:
error: expected primary-expression before ‘>’ token
364:14.30 962 | mozilla::DebugOnly rv = GetIsMediaRequest(&result);
364:14.31 | ^
And then there are yet more undefined type errors like this one:
364:14.36 ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Services.h:480:46:
error: ‘XPCOMService_GetHistory’ was not declared in this scope; did you
mean ‘XPCOMService_GetGfxInfo’?
364:14.36 480 | return already_AddRefed(XPCOMService_GetHistory());
364:14.36 | ^~~~~~~~~~~~~~~~~~~~~~~
364:14.36 | XPCOMService_GetGfxInfo
There are so many errors, but I'm hoping that it's a cascade: a single error that's triggering a whole cascade of follow-on errors, rather than a huge collection of unrelated errors. Time to dig in and find out.If I compare the code from ESR 78 it looks pretty similar, but I am a little surprised to see that the newly added Logging.h include is placed inside the namespace qualifiers:
using namespace mozilla::Compression;
namespace mozilla {
namespace scache {
#include "mozilla/Logging.h"
Although that does tally with the changes to the ESR 78 code, it doesn't match with the other ESR 91 files the Logging.h header is being included in, such as the EmbedLite code and elsewhere, so I'm going to move it out of the namespace and try again.This gives much better results: the majority of the errors are now gone and I'm just left with a couple of errors related to use of NS_LITERAL_STRING() and NS_LITERAL_CSTRING(). This has been a recurrent issue throughout this process since the gecko code now requires use of an _ns string suffix rather than one of the previous NS_LITERAL_STRING() macros.
Having replaced the two string literals with the annotations the code now compiles without error. But it means I have to do a complete build again, which means I have to regenerate the startupCache patch again. The commits have changed because the patch descriptions have changed as a result of being applied by the build process, but other than this I can use the same command to generate the patches as before:
$ git format-patch --start-number 79 -o ../rpm -N --no-signature --zero-commit
24f9ec2df3d3
The great news is that although this generates six patches, only the startUpCache patch is registering any changes, the others are all identical to the previous versions. This is a really good sign and exactly what I was hoping for.So I've updated the patches, added the changes to the commits, cleaned the entire source tree of cruft and now it's time for another full rebuild.
I want at least one full rebuild to go through without errors before I create this mythical pull request I've been going on about. It's frankly turning out to be a little more laborious to get it to happen than I was expecting. But it'll get there.
[...]
Now finally after a full day of compilation, the final full build has completed. I've copied over and installed the packages and a quick check suggests things are working as expected. The pages are returning correctly as mobile pages. The email app doesn't crash when you switch back from the homescreen. The WebView seems to be working correctly.
The browser does still segfault on exit and, after a discussion with Frajo, it's possible that this could be fixable. But this isn't a new issue and while it'd be great to have it fixed, it could take a while to figure out. So that's going to have to be something for the future.
The task now, finally, is to submit the pull request. In fact, not just one pull request, but all five of them for the various packages that have needed changes needed for supporting the new engine. After a bit of playing around on GitHub, here they are:
- https://github.com/sailfishos/gecko-dev/pull/162
- https://github.com/sailfishos/qtmozembed/pull/49
- https://github.com/sailfishos/sailfish-browser/pull/1074
- https://github.com/sailfishos/embedlite-components/pull/100
- https://github.com/sailfishos/sailfish-components-webview/pull/169
- Respond to the comments on the pull requests so they can be merged in. There are already some comments I need to address. Until they're merged in, they could still change a lot and ultimately, it's Jolla that has to decide whether or not to accept them. So they have to fulfil Jolla's requirements.
- Fix all the issues that have already come up in testing of the browser. It looks like many of these may be addressed by the user agent fix and the libhybris changes, but there are others. And I'm sure there will be many more besides.
- Fix the segfault on browser shutdown. This isn't new, but with the latest libhybris changes getting a clean shutdown feels closer than ever. It'd be so great if that could be achieved so it's worth spending a bit of time on it.
- Get the browser working on native devices. An important fix, mustn't forget this!
I'll return to this next weekend and I'll continue with fresh diary entries then. But right now, I plan to spend a bit of time breathing in and enjoying the fresh autumnal air.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
1 Sep 2024 : Day 337 #
Last night I kicked off the build to test the final arrangement of commits and patches that I hoped to convert into a pull request to the gecko-dev repository today. With the patches now stored in the gecko-dev rpm directory it means that the submodule mirror of upstream's gecko-dev source tree can be pulled in entirely unchanged and without any sailfish-specific commits. That's the whole point of the way the repository is structured: we pull in upstream's code without modification, apply our sailfish-specific patches to it and build the result.
This morning the build had finished and finished with a positive result. No glitches in the build process. But that doesn't mean it's built something that works.
But after transferring it over to my phone, installing the packages and running a few tests, I'm convinced that it's just the same as the previous version. So I conclude that I did successfully manage to reorganised the commits without affecting the final result.
At this point I thought I'd be ready to create the final pull request. But as I look through the patches I notice a few things that still need fixing. The first is to do with redundant commits. A couple of patches disable and then re-enable a flag, so they can be removed. There's also a commit that drops build optimisations to O1. I'd like to test it without this change so I've reverted this patch and will drop it entirely if it turns out not to be needed.
I also notice there are a few patches which aren't entirely required to get the browser working, but which do nevertheless provide important background functionality.
In fact, it was the discussion on the forum, particularly between Attah (attah) and Raine (rainemak) that alerted me to the fact that there are patches that haven't been applied but are needed, specifically related to user agent handling:
There are six patches like this that spring out as being likely needed, but which as yet aren't included in the build. Five of them apply without incident:
Unfortunately I wasn't able to get the changes to work correctly with my build engine. I suspect it's simply a case of me needing to reinstall the build engine to get a more recent version, but as this is quite a major task, it'll have to wait for now. I'm not as familiar as Matti is with the Rust toolchain in the Sailfish SDK, so I'm going to have to go back and discuss the details with him.
Finally, and this is really important, I received an email from Frajo concerning the libhybris changes. This also relates to the discussion on GitHub between Affe Null (affenull2345), Mal (mlehtima) and Frajo (krnlyng) in relation to getting the build running on Affe's Samsung Galaxy A40 port.
One of the outcomes of that discussion was to expose a further potential issue in libhybris. Once fixed Affe reported that the cover crash described in issue 1051 was no longer occurring. This cover crash has been a thorn in my side for several months now, so I'm really keen to know whether it does actually fix it or not.
Frajo provided me with some new libhybris rpm packages to test out that include these latest changes. Having installed them and tested the browser I find that... yes! The cover change no longer causes the browser to crash. This is good for having the cover working with the browser, but even more important is the fact it also resolves the issue with the email app and other WebView apps crashing when moving in and out of the app.
So I'm super-pleased with all this. I've sadly not had time to get involved in the topic in the forum over the last few days, but it's incredibly exciting to see all of the discussion and am thrilled at the effort people are putting in, not just into finding bugs, but also into resolving them.
Sadly I have to stop for the day today, but since I've made changes to the commits, especially to the optimisation of the build, I need to run a new build overnight from scratch before I can create my pull request. It'll add an extra day, but it's important to check everything is working correctly both at build and run-time after this change. Just one of the many reasons this has been such a slow process! Assuming the build passes, tomorrow morning I'll do a bit more testing of the changes and if all goes well I'll be able to create that pull request.
Tomorrow will be the last diary entry before I take a week off to attend RSECon24. I'm hoping I'll get the pull requests submitted by then, but either way there will still be more diary to write once I'm back in a week's time. So much to do!
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Comment
This morning the build had finished and finished with a positive result. No glitches in the build process. But that doesn't mean it's built something that works.
But after transferring it over to my phone, installing the packages and running a few tests, I'm convinced that it's just the same as the previous version. So I conclude that I did successfully manage to reorganised the commits without affecting the final result.
At this point I thought I'd be ready to create the final pull request. But as I look through the patches I notice a few things that still need fixing. The first is to do with redundant commits. A couple of patches disable and then re-enable a flag, so they can be removed. There's also a commit that drops build optimisations to O1. I'd like to test it without this change so I've reverted this patch and will drop it entirely if it turns out not to be needed.
I also notice there are a few patches which aren't entirely required to get the browser working, but which do nevertheless provide important background functionality.
In fact, it was the discussion on the forum, particularly between Attah (attah) and Raine (rainemak) that alerted me to the fact that there are patches that haven't been applied but are needed, specifically related to user agent handling:
attah: We also found that some of the UA handling looks to be missing a patch or two - so with those in, this may no longer be an issue.
[...]
rainemak: After we get the user-agent patch in place that should help in texture allocations (I think) and surely engine should start serving more mobile friendly pages.
[...]
rainemak: After we get the user-agent patch in place that should help in texture allocations (I think) and surely engine should start serving more mobile friendly pages.
There are six patches like this that spring out as being likely needed, but which as yet aren't included in the build. Five of them apply without incident:
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0059-sailfishos-gecko-Delete-startupCache-if-it-s-stale.patch
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0067-sailfishos-gecko-Hardcode-loopback-address-for-profi.patch
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0068-sailfishos-gecko-Start-using-user-agent-builder.-JB-.patch
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0083-sailfishos-gecko-dev-Disallow-page-zooming-if-the-me.patch
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0086-sailfishos-gecko-Add-preference-to-bypass-CORS-on-ns.patch
The final patch is messy: when I try to apply it, there are conflicts all the way down:
$ git am --3way \
../../../gecko-dev-project/gecko-dev/rpm/0061-sailfishos-locale-Get-12-24h-timeformat-setting-from.patch
In spite of this it turns out to be pretty simple to apply the changes manually, which I've now done. Since I did it as part of the git am process the correct description and attribution have already been automatically applied. So finally I can generate new patches from these commits and add them to the build configuration:
$ git format-patch \--start-number 79 -o ../rpm -N --no-signature --zero-commit bd026681f74dI also received a really helpful message from Matti Viljanen (direc85) giving suggestions for improving the build configuration to force the Rust portions to build with just a single process. The importance of this is that running Rust with multiple processes results in sporadic hangs during the build. That's a real pain because after leaving the build running overnight you don't want to discover in the morning it got stuck after the first hour and didn't make progress beyond that. If the Rust parts can be locked to a single process, it would allow the non-Rust parts (primarily C++ code, which still makes up the majority of the codebase) to run using multiple processes. That'd give us a nice speed-up.
Unfortunately I wasn't able to get the changes to work correctly with my build engine. I suspect it's simply a case of me needing to reinstall the build engine to get a more recent version, but as this is quite a major task, it'll have to wait for now. I'm not as familiar as Matti is with the Rust toolchain in the Sailfish SDK, so I'm going to have to go back and discuss the details with him.
Finally, and this is really important, I received an email from Frajo concerning the libhybris changes. This also relates to the discussion on GitHub between Affe Null (affenull2345), Mal (mlehtima) and Frajo (krnlyng) in relation to getting the build running on Affe's Samsung Galaxy A40 port.
One of the outcomes of that discussion was to expose a further potential issue in libhybris. Once fixed Affe reported that the cover crash described in issue 1051 was no longer occurring. This cover crash has been a thorn in my side for several months now, so I'm really keen to know whether it does actually fix it or not.
Frajo provided me with some new libhybris rpm packages to test out that include these latest changes. Having installed them and tested the browser I find that... yes! The cover change no longer causes the browser to crash. This is good for having the cover working with the browser, but even more important is the fact it also resolves the issue with the email app and other WebView apps crashing when moving in and out of the app.
So I'm super-pleased with all this. I've sadly not had time to get involved in the topic in the forum over the last few days, but it's incredibly exciting to see all of the discussion and am thrilled at the effort people are putting in, not just into finding bugs, but also into resolving them.
Sadly I have to stop for the day today, but since I've made changes to the commits, especially to the optimisation of the build, I need to run a new build overnight from scratch before I can create my pull request. It'll add an extra day, but it's important to check everything is working correctly both at build and run-time after this change. Just one of the many reasons this has been such a slow process! Assuming the build passes, tomorrow morning I'll do a bit more testing of the changes and if all goes well I'll be able to create that pull request.
Tomorrow will be the last diary entry before I take a week off to attend RSECon24. I'm hoping I'll get the pull requests submitted by then, but either way there will still be more diary to write once I'm back in a week's time. So much to do!
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.