812828 - "Open All in Tabs" doesn't work when only Library window is open with no other windows in background

Status: VERIFIED FIXED

(Toolkit :: Places, defect)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Regression window (mozilla-inbound-win64-pgo)
Good:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win64-pgo/1352901940/

Bad:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win64-pgo/1352912619/

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c44db6486c0d&tochange=a6305a66f8c1

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Regression window (mozilla-inbound-win32)
Good:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1352908422/

Bad:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1352911602/

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3c17914da5d6&tochange=a6305a66f8c1

Comment 2

[Alice0775 White]

An error in error console as follows:

Error: TypeError: aWindow is null
Source file: resource://gre/modules/PrivateBrowsingUtils.jsm
Line: 24


Suspected bug 723005

Comment 3

[Marco Bonardo [:mak]]

we should likely assume not-in-pb-window if there's no open browser window, unless windows opened from a pb window should gain some pb flag... Josh?

Comment 4

[Marco Bonardo [:mak]]

we should likely assume not-in-pb-window if there's no open browser window, unless windows opened from a pb window should gain some pb flag... Josh?

Comment 5

[Josh Matthews [:jdm]]

Agreed.

Comment 6

[Josh Matthews [:jdm]]

Attachment: If there's no active window when opening targets in tabs, assume we don't want private browsing.

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 683124 [details] [diff] [review]
If there's no active window when opening targets in tabs, assume we don't want private browsing.

Review of attachment 683124 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the following change

::: browser/components/places/src/PlacesUIUtils.jsm
@@ +550,5 @@
>      var urls = [];
>      for (let item of aItemsToOpen) {
> +      urls.push(item.uri);
> +      if (browserWindow && PrivateBrowsingUtils.isWindowPrivate(browserWindow)) {
> +        continue;

So, now that I look at this again, the pb status is not going to change through the loop, so it should be moved outside of it like

if (!pb) {
  for () {}
}

Comment 8

[Marco Bonardo [:mak]]

her I missed the .push(), so just move out the pbwin && ispbwin to a temp var and check the var in the loop.

Comment 9

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a401cd8c40e4

This may need backporting depending on when the merge occurs,

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a401cd8c40e4

Comment 11

[Marco Bonardo [:mak]]

Comment on attachment 683124 [details] [diff] [review]
If there's no active window when opening targets in tabs, assume we don't want private browsing.

[Approval Request Comment]
Bug caused by (feature/regressing bug #):  	723005
User impact if declined: Open all in tabs doesn't from from the Library if no browser window is open
Testing completed (on m-c, etc.): Manual testing.
Risk to taking this patch (and alternatives if risky): None, just a null check
String or UUID changes made by this patch: None.

Comment 12

[Alex Keybl [:akeybl]]

Not a severe enough regression to track, but approving for Aurora 19 nonetheless.

Comment 13

[Josh Matthews [:jdm]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6cbf0193153f

Comment 14

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

VERIFIED on latest hourly Nightly builds

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:20.0) Gecko/20.0 Firefox/20.0
buildID: 20121120180539
Source: Built from http://hg.mozilla.org/integration/mozilla-inbound/rev/0dc53785ef12


Thank you! :)

Comment 15

[u279076]

Is this something that could be automated in testsuite?

Comment 16

[Ioana (away)]

Verified as fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 (20130109111322).