"Common myths about private browsing" link is not clickable when Firefox is started with --private-window
Categories
(Firefox :: Private Browsing, defect, P2)
Tracking
()
People
(Reporter: botond, Unassigned)
References
(Regression)
Details
(Keywords: regression)
STR
- Launch Firefox from the command line with
--private-window
as an argument, such that the first / only window that opens is the private window. - Try to click on the "Common myths about private browsing" link near the bottom of the start page.
Expected results
The link is clickable.
Actual results
The link is not clickable.
Reporter | ||
Comment 1•5 years ago
|
||
I got a regression range with mozregression --arg='--private-window'
:
(Note that in earlier builds the link text was "Learn more about Private Browsing", but the bug affected it the same.)
Bug 1459204 perhaps?
Unfortunately there are no inbound builds for that time period.
Comment 2•5 years ago
|
||
Looks like it, the content console shows: "ReferenceError: RPMIsWindowPrivate is not defined" for line 8 of aboutPrivateBrowsing.js on current nightly.
There are some other issues on the page, too (e.g. the background colour of the search field is purple instead of white - I expect other code not executing is tripping that. It looks like this is some kind of race where we don't get to run the code in https://searchfox.org/mozilla-central/rev/428cf94f4ddfb80eebc6028023a7d89eb277791b/toolkit/components/remotepagemanager/RemotePageManagerChild.jsm#15 in time. Dave, as you (IIRC) wrote large parts of the RPM infrastructure, any idea as to what might be causing such a race condition, and/or how we'd fix it?
Comment 3•5 years ago
•
|
||
Workaround: refresh the page.
This might not be super serious if it's only for manual use of this commandline arg, esp. given the workaround, but if this is still exposed using the taskbar menu on windows that makes it a little bit more problematic...
Comment 4•5 years ago
|
||
The problem is that this is getting run too late: https://searchfox.org/mozilla-central/source/browser/components/BrowserGlue.jsm#1661
Comment 5•5 years ago
|
||
This does seem to affect the taskbar context menu case on Windows, unfortunately (if Firefox is not already up).
(In reply to Dave Townsend [:mossop] (he/him) from comment #4)
The problem is that this is getting run too late: https://searchfox.org/mozilla-central/source/browser/components/BrowserGlue.jsm#1661
I think in the past we've been reluctant to centralize the list of remote pages so we don't need to pass this knowledge to the child, because it makes it less "pluggable" and spreads that around. I suspect we also don't want to pay startup costs for the IPC / registration by moving all the registration to Very Early On Startup for all these pages (I suspect others may be affected if loaded as the initial page through session restore or when set as the homepage). The third option I see is updating RemotePages to also inject into existing windows (handwave as to how that works), and to add guards and an event in the individual pages so they deal with the edgecase. That's a little messy but avoids some of the perf downsides and centralization... We could potentially abstract it behind a generic script these pages could load...
Dave, how would you feel about these solutions, and/or are there obvious other ones that I'm missing?
Comment 6•5 years ago
|
||
Well. I think the security team are very much interested in centralizing the list of registered RPM pages (though not necessarily the code that handles them). I don't think the cost of doing so is high, we just need to set the list of registered urls in the shared memory and then deal with loading the actual handling code when we encounter one (this would probably save us some work on startup as I'm betting we do that multiple times currently).
But I don't think we need to figure that out here. I'd just ensure the privatebrowsing case is initialised here: https://searchfox.org/mozilla-central/source/browser/components/BrowserContentHandler.jsm#527
Comment 7•5 years ago
|
||
Christoph, any chance you could have a look at this, please? Thanks!
Updated•5 years ago
|
Updated•3 years ago
|
Description
•