Closed Bug 819561 Opened 7 years ago Closed 7 years ago

pages opened by AdBlock Plus during installation should open in the same PB window

Categories

(Firefox :: Private Browsing, defect)

20 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jbecerra, Unassigned)

References

Details

(Whiteboard: [testday-20121207])

Using latest birch nightly, when you install an add-on like AdBlock Plus (restartless) from within a PB window it will launch a page with information but it will launch the page in the public window. It should open it in the same window from which it was installed.

Steps:
1. Open a private browsing window using command-shift-p
2. Go to addons.mozilla.org
3. Look for adblock plus and install it

Expected: Whatever pages are launched from installing an add-on are launched within the PB window.

Actual: At the end of the installation this page opens in the non-private window: chrome://adblockplus/content/ui/firstRun.xhtml
Whiteboard: [testday-20121207]
Well, this depends on how the add-on opens its new window.  There isn't really a single solution to this problem.  CCing Wladimir to ask him about this.
Summary: pages opened by addons during installation should open in the same PB window → pages opened by AdBlock Plus during installation should open in the same PB window
Adblock Plus uses nsIWindowWatcher.getWindowEnumerator() to get all windows on startup - it then applies to the browser windows in that order. Whichever window comes first will be the one where the first-run page is opened. Not sure why I chose that enumerator, I guess that replacing it by nsIWindowMediator.getZOrderDOMWindowEnumerator() (plus compensating for bug 156333) should also work fine and fix the issue here as a side-effect.
Review created for the fix: http://codereview.adblockplus.org/9318048/
The fix has been committed: https://hg.adblockplus.org/buildtools/rev/656466cec3c0. This issue doesn't seem to require an urgent release, the fix will be included in the next regular release however.
(In reply to comment #2)
> Adblock Plus uses nsIWindowWatcher.getWindowEnumerator() to get all windows on
> startup - it then applies to the browser windows in that order. Whichever
> window comes first will be the one where the first-run page is opened. Not sure
> why I chose that enumerator, I guess that replacing it by
> nsIWindowMediator.getZOrderDOMWindowEnumerator() (plus compensating for bug
> 156333) should also work fine and fix the issue here as a side-effect.

Hmm, I think you should be able to use RecentWindow.getMostRecentBrowserWindow({private: true}) if you want a recent private window (similarly with {private:false}).

http://mxr.mozilla.org/mozilla-central/source/browser/modules/RecentWindow.jsm#28
(In reply to :Ehsan Akhgari (Away 2/7-2/15) from comment #5)
> Hmm, I think you should be able to use
> RecentWindow.getMostRecentBrowserWindow({private: true}) if you want a
> recent private window (similarly with {private:false}).

No, Adblock Plus needs all windows, it should merely start with the most recent window. Also, IMHO Adblock Plus has no way of knowing which window triggered the installation - whether it was a private browsing window or not. Not to mention that this code needs to work in all applications whereas RecentWidnow.jsm seems to be Firefox-specific. My code essentially does the same thing as this getMostRecentBrowserWindow() function anyway.

Adblock Plus 2.2.3 has been released, can this be considered fixed?
Yes, thanks!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.