1538546 - Private browsing opt-in should not be shown for extensions with `incognito: "not_allowed"`

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[stoically]

Tested with Firefox Developer Edition / Beta 67.0b4 on Linux

What did you do? (steps to reproduce)

• Install a WebExtension that defines incognito: not_allowed in the manifest.json
• The popup "Extension has been added to Firefox" is shown including the "Allow this extension to run in Private Browsing" checkbox
• Tick the checkbox

What happened? (actual results)

• The extension is now also available in Private Browsing
• In the extension preferences is no "Run in Private Windows" checkbox available

What should have happened? (expected results)

• The Private Browsing checkbox when installing should not be shown
• It should not be possible for the user to overwrite not_allowed

Comment 1

[Kris Maglione [:kmag]]

Bug 1533172 added the appropriate checks for the prompts that it added. I suppose we need to fix the rest to use the same logic, but I guess we should just check the PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission everywhere, rather than explicitly checking the extension type, incognito == "not_allowed", and extensions.allowPrivateBrowsingByDefault" everywhere.

Comment 2

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

Attachment: Bug 1538546 - Hide incognito checkbox in addon-installed notification for not_allowed extensions. r?mixedpuppy!

Comment 3

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

Attachment: Bug 1538546 - Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission instead of the incognito manifest property. r?kmag!,mixedpuppy

Depends on D24719

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b51e70c12fb
Hide incognito checkbox in addon-installed notification for not_allowed extensions. r=mixedpuppy,kmag
https://hg.mozilla.org/integration/autoland/rev/96def3091254
Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission instead of the incognito manifest property. r=kmag,mixedpuppy

Comment 5

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9053290 [details]
Bug 1538546 - Hide incognito checkbox in addon-installed notification for not_allowed extensions. r?mixedpuppy!

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1380809
• User impact if declined: Some extensions that do not need or should not have permission toggling will have that capability. This results in UI that seems to fail.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: STR in comment 0
• List of other uplifts needed: Bug 1537542
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): minimal changes with automated tests
• String changes made/needed: non
• Do you want to request approval of these patches as well?: on

Comment 6

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9053291 [details]
Bug 1538546 - Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission instead of the incognito manifest property. r?kmag!,mixedpuppy

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1380809
• User impact if declined: See request for other patch in this bug.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:
• Do you want to request approval of these patches as well?: on

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/2b51e70c12fb
https://hg.mozilla.org/mozilla-central/rev/96def3091254

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Let's have QA confirm that the fix works in Nightly with the STR in comment #0 before evaluating the uplift, thanks.

Comment 9

[Vlad Jiman]

Verified as fixed on Nightly 68.0a1 Build ID: 20190328173141 using steps from the description and a test extension having "incognito":"not_allowed".
The 'Allow this extension to run in Private Browsing' checkbox is not shown for these extensions. Testing has been done using Windows 10 x64.

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9053291 [details]
Bug 1538546 - Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission instead of the incognito manifest property. r?kmag!,mixedpuppy

P1, fix for our extensions in incognito mode support which is a 67 feature, verified by QA on Nightly, approved for 67 beta 7, thanks.

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8935c8db2301
https://hg.mozilla.org/releases/mozilla-beta/rev/381133904780

Comment 12

[Bogdan Maris, Desktop QA]

Setting qe-verify+ again for verification in 67.0b7.

Comment 13

[Vlad Jiman]

Verified using 67.0b7 on Windows 10 x64, using the same steps as in comment #9, no problems have been found and the checkbox is not shown on the test extension containing ("incognito":"not_allowed"), as expected.