`about:compat` does not survive extension updates
Categories
(Web Compatibility :: Interventions, defect, P3)
Tracking
(Not tracked)
People
(Reporter: denschub, Unassigned)
References
Details
- Take the Release build of Firefox with WebCompat 4.3.2 built-in
- Update to webcompat 4.3.3 by either installing https://bugzilla.mozilla.org/attachment.cgi?id=9077742 (this XPI is signed as a system extension), or by following https://bugzilla.mozilla.org/show_bug.cgi?id=1564974#c9 and have balrog deliver the update (the latter approach likely is invalid soon when we take the xpi off the staging area)
- Restart the browser
- Try to access
about:compat
You will see an Invalid URL page ("Hmm. That address doesn’t look right."), which never resolves itself, even after multiple browser restarts. :(
Reporter | ||
Comment 1•5 years ago
|
||
Tom, any idea on how we could fix that?
Reporter | ||
Comment 3•5 years ago
|
||
Well, yes, "TypeError: docShell.failedChannel is null" from NetErrorChild.jsm:697:30
, but unfortunately, that's not really helpful, so I left it out. Nothing that could point in any direction. :(
Comment 4•5 years ago
|
||
Alright, I've been able to figure this out by reproducing the problem using a local nightly with unsigned XPIs with some debug-console.logs in them.
There are two problems. The first is that the addon does not set up its resource:
URLs properly for the case where a user-installed (or Balrog-installed) XPI file is used from their profile, instead of from the bundled version with Firefox. Fixing this isn't tough, we just have to change a chunk in aboutPage.js to point to the relative location of the files in the XPI:
resProto.setSubstitution(
ResourceSubstitution,
Services.io.newURI("about-compat/", null, rootURI)
);
This removes the need to bother with a jar.mn
, so we can remove that as well. I've tested this on local Linux and Fennec builds, both with their bundled addon, and also with a replacement XPI installed on top of that to simulate a Balrog-delivered update.
The second problem is that the user will have to restart their running browser before about:compat will work again. The reason for this is rather obnoxious:
- the addon uses a process script to register the about-page handler for about:compat
- this registration expects the about-compat files to be in the XPI's root directory, but we recently moved them to /about-compat
- as such it breaks because the files are not where it expects them (though the addon does continue to work)
- it is never un-registered by the addon when it unloads (I recall that being deemed to be overcomplicated at review-time when about:compat was implemented)
- it cannot be simply replaced by a new registration (with a new CID or the old one); direct re-registrations appears to be ignored
- it cannot be unregistered, because a reference to the old registration is required, and the newly-loading addon has no way to get one.
The net result is that if the user gets a Balrog update, they will need to restart the browser for it to pick up on the new paths for the about:compat files.
Our options are simple:
- ignore this, and ask users to restart if they run into such temporary breakage
- move the about-compat files back to the root directory of the XPI.
If we decide to pick #2 for now, and try to move the files around again in the future, we can also fix the addon to unregister the about-page handler as it unloads. This should not be too difficult, it would involve some message-passing.
I have a candidate patch ready which moves the files back, and fixes the resource-URL issue. I've tested it on Fennec and Linux, and it now seems to work fine with and without a restart.
Comment 6•5 years ago
|
||
I believe so, but I've left it up to Dennis as to when we actually do land the fix.
Hi Tania, Andrei, can we add this scenario to our QA testing?
Comment 8•5 years ago
|
||
OK, one last question -- what's the risk for landing this fix via a balrog update? Will it work?
Comment 9•5 years ago
|
||
If we just want to ship the jar-file fix on the version 4.4.0 of the addon that's currently in m-c, and not all of the other stuff including the files moving around into an about-compat subdirectory, then the risk is quite low. At least, I've manually tested that just installing a version 4.4.1 of the XPI with only that fix into my profile doesn't break anything, before or after a Firefox restart.
However, if we want to also Balrog-deploy a version including the other fixes on GitHub (with the ones which move the files around), then we run into the problem where users will have to restart Firefox before about:compat will work again. That's the only risk I'm aware of.
Reporter | ||
Comment 10•5 years ago
|
||
I've left it up to Dennis as to when we actually do land the fix.
I have no strong opinion here. If noone else does it, I'll make sure to build a patch merging everything we have right now (maybe including the new console logging, if it's done by then) by the end of the week!
Comment 11•5 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #7)
Hi Tania, Andrei, can we add this scenario to our QA testing?
Of course. Emil is currently looking at the information available here and will add a regression test.
Comment 12•5 years ago
|
||
Added a regression test case to our Web Compatibility test suite.
Hi Andrei, this change/fix is also being pushed in SAO update: webcompat system addon 5.0.2. I am sure your team is also testing that fix. It's a critical one to validate. I just wanted to flag it again in case the test case mentioned in c12 isn't enough to test the SAO fix. Thanks!
Comment 14•5 years ago
•
|
||
Hi Ritu, per Bug 1568636 Comment 26, this has been verified by the Web Compatibility team.
Updated•4 years ago
|
Comment 15•2 years ago
|
||
Dennis, could you confirm if this is still something we need to be worried about?
Reporter | ||
Comment 16•2 years ago
|
||
I did some tests, and surprisingly, it works now. I didn't run the full staging-rollout process, but if it works with a locally built .xpi and an xpi built by CI, I see no reason why this shouldn't work otherwise.
Changes are even visible on about:compat
without restarting the browser. So, uh, good. :)
Description
•