1527127 - set permission on initial upgrade to incognito feature

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P1)

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1527127 set permission for enabled extensions on initial upgrade

When a user first upgrades to a version where incognito is enabled,
add the permission to all enabled extensions. This preserves user workflow
and addresses other issues (e.g. proxy). This will only happen once.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1527127 update addon panel text for private windows

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: about:addons banner

Text change on about:addons

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: private window first run

Comment 5

[Shane Caraveo (:mixedpuppy)]

Attachment: post install panel with permission

Comment 6

[Meridel [:meridel]]

Comment on attachment 9044037 [details]
post install panel with permission

"Learn More" does not work as a button, unfortunately. It isn't clear what the user will learn more about since it is so far from the "Allow this extension to run in Private Windows" string. We need to be able to incorporate the link/string into the main message. Linking via button won't work.

Comment 7

[Meridel [:meridel]]

Comment on attachment 9044034 [details]
about:addons banner

Looks good!

Comment 8

[Meridel [:meridel]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)

Created attachment 9044035 [details]
private window first runhttps://bugzilla.mozilla.org/enter_bug.cgi

Looks good!

Comment 9

[emanuela [ux]]

Comment on attachment 9044034 [details]
about:addons banner

Looks good.

Comment 10

[emanuela [ux]]

Question: why the message bar is sticky?
Can we unstick it? It's pretty long, and I'm afraid it will annoying to the end users who already act on it. If it's something complicated, I'm ok to track this in a separate bug.

Comment 11

[emanuela [ux]]

Comment on attachment 9044037 [details]
post install panel with permission

It looks good. 

If possible, I'll add to the checkbox ~8/16px of margin-top. It needs a bit more air on top.

Comment 12

[Shane Caraveo (:mixedpuppy)]

(In reply to emanuela [ux] from comment #10)

Question: why the message bar is sticky?
Can we unstick it? It's pretty long, and I'm afraid it will annoying to the end users who already act on it. If it's something complicated, I'm ok to track this in a separate bug.

I'm not clear what this is about. What message bar?

Comment 13

[emanuela [ux]]

Attachment: messagebar.mov

The message bar it's the banner in about:addons.

I attached a video that shows what I described in the other comment.

Comment 14

[Shane Caraveo (:mixedpuppy)]

(In reply to emanuela [ux] from comment #13)

Created attachment 9044199 [details]
messagebar.mov

The message bar it's the banner in about:addons.

I attached a video that shows what I described in the other comment.

That is the way that block works. It could change in a followup, it will affect all usage of that block.

Comment 15

[emanuela [ux]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #14)

(In reply to emanuela [ux] from comment #13)

Created attachment 9044199 [details]
messagebar.mov

The message bar it's the banner in about:addons.

I attached a video that shows what I described in the other comment.

That is the way that block works. It could change in a followup, it will affect all usage of that block.

I think I never noticed cos the block was never so tall. I'll create a followup bug. This small issue is not a blocker.

Comment 16

[Shane Caraveo (:mixedpuppy)]

Attachment: post install panel with permission.png

more padding on top of checkbox (using margin: 8px 4px 2px;)

Comment 17

[emanuela [ux]]

Comment on attachment 9045294 [details]
post install panel with permission.png

Good for me! Thank you, Shane

Comment 19

[David Durst [:ddurst]]

How does this work with bug 1513344? Do we send privateBrowsingAllowed "on" as soon as the doorhanger value is set (or not set)?

Comment 20

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to David Durst [:ddurst] from comment #19)

How does this work with bug 1513344? Do we send privateBrowsingAllowed "on" as soon as the doorhanger value is set (or not set)?

Bug 1513344 doesn't yet cover this, we filed bug 1529347 as the follow up in which we will record an additional telemetry event when a user is enabling an extension on PB windows from the install doorhanger.

Comment 21

[Shane Caraveo (:mixedpuppy)]

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

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3063f37eb7b2
set permission for enabled extensions on initial upgrade r=aswan
https://hg.mozilla.org/integration/autoland/rev/30aad4a580eb
update addon panel text for private windows r=flod,Gijs,mstriemer,rpl

Comment 23

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3063f37eb7b2
https://hg.mozilla.org/mozilla-central/rev/30aad4a580eb

Comment 24

[Ton [:Tonnes]]

(In reply to Cosmin Sabou [:CosminS] from comment #23)

https://hg.mozilla.org/mozilla-central/rev/30aad4a580eb

+<!ENTITY addonPostInstall.incognito.label "Allow this extension to run in Private Windows.">

I’m afraid this is an option, and hence should not end with a trailing period. The same may apply to addonPrivateBrowsing.learnMore.label and private-browsing-learn-more (see legacy-learn-more).

Also see https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Menu_and_control_labels.

Comment 25

[Francesco Lodolo [:flod]]

(In reply to Ton [:Tonnes] from comment #24)

(In reply to Cosmin Sabou [:CosminS] from comment #23)

https://hg.mozilla.org/mozilla-central/rev/30aad4a580eb

+<!ENTITY addonPostInstall.incognito.label "Allow this extension to run in Private Windows.">

I’m afraid this is an option, and hence should not end with a trailing period. The same may apply to addonPrivateBrowsing.learnMore.label and private-browsing-learn-more (see legacy-learn-more).

Also see https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Menu_and_control_labels.

That document shouldn't be used as point of reference for en-US copy. If such a reference can be created, it should live here and be done by the team in charge of copy
https://design.firefox.com/photon/copy/punctuation.html

Comment 26

[Francesco Lodolo [:flod]]

(In reply to Ton [:Tonnes] from comment #24)

(In reply to Cosmin Sabou [:CosminS] from comment #23)

https://hg.mozilla.org/mozilla-central/rev/30aad4a580eb

+<!ENTITY addonPostInstall.incognito.label "Allow this extension to run in Private Windows.">

I’m afraid this is an option

For clarity, it's a checkbox (and yes, normally they don't have a closing period).

Comment 27

[CosminB]

Attachment: Bug1527127.zip

This issue is verified as fixed on Firefox 67.0a1 (20190304215035) under Win 7 64-bit and Mac OS X 10.14.1.

After updating Nightly from an older version to the latest one, with some installed extensions, the enabled extensions are automatically granted in PB.

The post install panel is displayed, like in the Comment 17.

Please see the attached video and screenshot.

The banner from about:addons is permanently displayed while scrolling, this issue will be tracked in a separate bug.

Comment 28

[Shane Caraveo (:mixedpuppy)]

(In reply to Ton [:Tonnes] from comment #24)

The same may apply to addonPrivateBrowsing.learnMore.label and private-browsing-learn-more (see legacy-learn-more).

Given it's a complete sentence, not on an option, I think the period is appropriate. I'll let Flod & Meridel decide.

See attachment 9044034 [details] and attachment 9044035 [details].

Comment 29

[Francesco Lodolo [:flod]]

(That's a copy question, not l10n)

Comment 30

[Meridel [:meridel]]

No period at the end of the link. Following patterns established in other parts of the product, including new Private Windows new tab.