839746 - UI modules should work when global pb is enabled and pb is not supported

Status: RESOLVED FIXED

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

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

Does this make sense to you all that in 1.14 if `'private-browsing': true` is not used in package.json file and the add-on is installed on Fx < 20, which uses global pb, then all private windows will be ignored?

This changes the default behavior on Fx < 20 when global pb is enabled, but it feels like a natural progression given the change we're making with per-window pb.

Comment 2

[Jeff Griffiths (:canuckistani) (:⚡︎)]

IIRC that's what we agreed on. +1 unless there is some technical issue I'm blissfully ignorant of.

Comment 3

[Dave Townsend [:mossop]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #1)
> Does this make sense to you all that in 1.14 if `'private-browsing': true`
> is not used in package.json file and the add-on is installed on Fx < 20,
> which uses global pb, then all private windows will be ignored?

So you mean when global private browsing is enabled on Fx < 20 add-ons won't see any windows? Yes that follows I think.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Sounds good to me. 

Maybe property name can be made more descriptive though, like `supports-private-browsing: true, { supports: { private-browsing: true } }` or something, I'll let native ENG speakers bike-shed / decide on proper naming.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

BTW: Whatever we end up adding for private browsing in package.json we should consider doing for add-on page too. At the moment it's kind of stupid as one just needs to `require("addon-page")` to enable it.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #5)
> BTW: Whatever we end up adding for private browsing in package.json we
> should consider doing for add-on page too. At the moment it's kind of stupid
> as one just needs to `require("addon-page")` to enable it.

See https://bugzilla.mozilla.org/show_bug.cgi?id=820582#c12

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Erik, is this necessary, since we're not gonna get it in for 1.14? Since 1.15 will never be targeting Firefox versions with global private browsing, it doesn't seem necessary.

Comment 8

[Erik Vold [:erikvold] (please needinfo? me)]

I think it is necessary, otherwise it may make the "permissions" key implemented in the package.json file look contrived.

Comment 9

[Dave Townsend [:mossop]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #8)
> I think it is necessary, otherwise it may make the "permissions" key
> implemented in the package.json file look contrived.

Can you explain further. 1.15 will never be in a situation where global private browsing will be enabled so how can this be necessary?

Comment 10

[Erik Vold [:erikvold] (please needinfo? me)]

Good example add-on https://github.com/canuckistani/panel-pwpb

Comment 11

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Dave Townsend (:Mossop) from comment #9)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #8)
> > I think it is necessary, otherwise it may make the "permissions" key
> > implemented in the package.json file look contrived.
> 
> Can you explain further. 1.15 will never be in a situation where global
> private browsing will be enabled so how can this be necessary?

1.14 will be in this situation though, and the package.json flag has landed in 1.14

Comment 12

[Erik Vold [:erikvold] (please needinfo? me)]

At the moment if we don't land a patch for this then things like widget will work on global pb windows and panel will not, so the way things stand is not good.

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

I thought when global pb was enabled the old window was closed and a new one was opened, but it turns out all windows but one are closed, the one left is re-purposed to be a private window..  so I can't easily implement this bug.

So I will leave the behavior on Fx19 unchanged, and the pb mode of a window will not matter, Fx19 will be gone soon enough and this won't be an issue anymore so I don't think it is worth the effort.

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/881

Pointer to Github pull-request

Comment 15

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #14)
> Created attachment 727530 [details]
> Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/881
> 
> Pointer to Github pull-request

Basically nothing was being tested when global private browsing was enabled, so I had no tests to use for this situation, and my change made no tests fail..

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #14)
> Created attachment 727530 [details]
> Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/881
> 
> Pointer to Github pull-request

I get an error running cfx testaddons against Firefox 19:

ModuleNotFoundError: uunable to satisfy: require(.test-window-utils-global-private-browsing) from /home/kwierso/mozilla/evsdk/test/addons/private-browsing-supported/main.js:15

Comment 17

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 727530 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/881

just thinking Alex might be able to look at this first.

Comment 18

[Alexandre Poirot [:ochameau]]

Comment on attachment 727530 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/881

Thanks again Erik for you hard work on pb!

Comment 19

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/888

Pointer to Github pull-request

Comment 20

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/888#attch-to-bugzilla

Pointer to Github pull-request

Comment 21

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 728431 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/888

Adding many reviewers since this needs to land asap, but I really need only one review who ever can do it first.

Comment 22

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/3ca318538240ebaebab4f2e9dc11318c4139bc3f
Bug 839746: global private windows (Fx <=19) are not ignored, per widnow private windows (on Fx > 19) still are

https://github.com/mozilla/addon-sdk/commit/eed3c68aa20dd93677ebc7c30eba62d3a585c482
Merge pull request #888 from Gozala/bug/indipendent-panel@839746

Bug 839746 - global private windows (Fx <=19) are not ignored r=@ZER0

Comment 23

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/889

Pointer to Github pull-request

Comment 24

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/886

Pointer to Github pull-request

Comment 25

[[github robot]]

Commits pushed to stabilization at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/9d9d14df1ece69f315faf652683473d700d73023
Bug 839746: global private windows (Fx <=19) are not ignored, per widnow private windows (on Fx > 19) still are
(cherry picked from commit ad44c4bc411d48af3cf001703729cf928fcc3ee9)

https://github.com/mozilla/addon-sdk/commit/11d3e8b298cf0f6f772c1cdc16e0e2c71744b185
Bug 839746: added tests for tabs/utils getTabs, added tests for panel/window getWindow, and page-mods in global pb
(cherry picked from commit 529de8625b6d9d3ea6b2051c34c8291f12ef4453)

Conflicts:

	test/test-panel.js

https://github.com/mozilla/addon-sdk/commit/a2c4383ae71a0c89a6cf682cb7122fc6d27886a3
Merge pull request #894 from erikvold/839746-4-stabilization

Bug 839746 for stabilization: global private windows are not ignored r=@gozala

Comment 26

[[github robot]]

Commit pushed to stabilization at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b1467d94ec33eddfe3a9440be485d731501c75a8
Merge pull request #895 from erikvold/stabilization

Bug 839746 Actually disable tests for global PB if not supported a=@erikvold

Comment 27

[Jeff Griffiths (:canuckistani) (:⚡︎)]

We have some Firefox 22 work to be done, but this bug no longer blocks SDK 1.14 release.

Comment 28

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/e90f232e04478f5a2b9cd4b700dd989c8e0351e0
Bug 839746: added tests for tabs/utils getTabs, added tests for panel/window getWindow, and page-mods in global pb
(cherry picked from commit 529de8625b6d9d3ea6b2051c34c8291f12ef4453)

Conflicts:

	test/test-page-mod.js
	test/test-panel.js

https://github.com/mozilla/addon-sdk/commit/9bdabb487a0834a72395c51129a9536e00572a14
Merge pull request #892 from erikvold/839746-4-master

Bug 839746: added tests for tabs/utils and panel r=@gozala

Comment 29

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Can we close this bug now ?

Comment 30

[Jeff Griffiths (:canuckistani) (:⚡︎)]

yup.

Comment 31

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fcf657184b70e1efc7563cdcf50eadf6b0940953
Merge pull request #889 from Gozala/bug/remove-method-workaround@839746

Bug 839746 - Remove workaround that was hard to uplift. r=@zer0

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to [github robot] from comment #31)
> Commit pushed to master at https://github.com/mozilla/addon-sdk
> 
> https://github.com/mozilla/addon-sdk/commit/
> fcf657184b70e1efc7563cdcf50eadf6b0940953
> Merge pull request #889 from Gozala/bug/remove-method-workaround@839746
> 
> Bug 839746 - Remove workaround that was hard to uplift. r=@zer0

Does this need uplifted to Aurora, Irakli?

Comment 33

[[github robot]]

Commit pushed to integration at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fcf657184b70e1efc7563cdcf50eadf6b0940953
Merge pull request #889 from Gozala/bug/remove-method-workaround@839746

Comment 34

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I don't think needinfo here is still relevant.