1224863 - The windows module does not list the first window when “Never Remember History” is set

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Pierre Bertet]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20151114030404

Steps to reproduce:

- Open the Preferences
- Go to Privacy
- Set “Never Remember History”

- Allow private browsing this in package.json of your addon:

"permissions": { "private-browsing": true }

- Check the length of require('sdk/windows').browserWindows


Actual results:

The first window is not listed in the windows module, while subsequent windows are triggering the “open” event and are accessible.

require('sdk/windows').browserWindows.length // 0


Expected results:

The window should be listed.

require('sdk/windows').browserWindows.length // 1

Comment 1

[Kestrel]

Same problem here, it's a regression caused by Bug 1160676.

Comment 2

[PerfectSlayer]

Attachment: A fix to include private browsing window at startup

Hi,

Here is a fix to the issue.
Sorry but I didn't figure out how to add an automated test to cover it.

I hope it could help.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8782043 [details] [diff] [review]
A fix to include private browsing window at startup

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

Looks good to me!  Thanks!

::: lib/sdk/windows/firefox.js
@@ +182,4 @@
>      return new Window(domWindow);
>  }
>  
> +for (let domWindow of windows(null, {includePrivate: supportPrivateWindows})) {

I checked that supportPrivateWindows is set based on the manifest permission.  So I believe this will correctly respect the addon permissions.

Comment 4

[PerfectSlayer]

Thanks for the review!

So what's the next step to push/publish the patch?

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Make addon-sdk windows module include private browsing windows in browserWindows. r=bkelly

A couple things:

1) The patch needs to be a diff from our repository root.  In this case you were missing the addon-sdk/source part of the page.  I fixed this up for you.
2) The patch needs a good commit message with the bug number and your email address specified.  I also fixed this.
3) We push the patch to our try server to verify it doesn't break any automated tests.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4898fe49f5f2

If that comes back green or only with known failures, then we can add the checkin-needed keyword to get it landed in the tree.

Some links that might help with these steps:

https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial
https://wiki.mozilla.org/Build:TryServer

Comment 6

[PerfectSlayer]

Thanks for your fixes and the explanations.

The build is done. I'll let you check and deal with this bug report.
Thanks again for you help.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/5e10edf2ce87
Make addon-sdk windows module include private browsing windows in browserWindows. r=bkelly

Comment 8

[Pierre Bertet]

Awesome!

Just one thing: the commit author appear to be me, but I just reported the bug. PerfectSlayer is the author of the fix.

Comment 9

[PerfectSlayer]

Nice catch Pierre!
But I can't fix it, it's Ben Kelly who send it.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Oh, I am so sorry for that!  I will back-out and re-land with correct author.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Make addon-sdk windows module include private browsing windows in browserWindows. r=bkelly

This should have the patch author information fixed.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/830427b0d451
Backed out changeset 5e10edf2ce87 for landing with the wrong patch author.
https://hg.mozilla.org/integration/fx-team/rev/4cfc2de918bc
Make addon-sdk windows module include private browsing windows in browserWindows. r=bkelly

Comment 13

[PerfectSlayer]

Thanks Ben for keeping me as author.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4cfc2de918bc