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

VERIFIED FIXED in Firefox 67

Status

()

defect
P1
normal
VERIFIED FIXED
5 months ago
5 months ago

People

(Reporter: stoically, Assigned: rpl)

Tracking

(Blocks 1 bug, {regression})

67 Branch
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 verified, firefox68 verified)

Details

Attachments

(2 attachments)

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
Component: General → Add-ons Manager
Product: WebExtensions → Toolkit
Summary: incognito not_allowed in the manifest can be overwritten by user choice → Private browsing opt-in should not be shown for extensions with `incognito: "not_allowed"`
Version: Firefox 67 → 67 Branch

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.

No longer blocks: 1511636
Attachment #9053291 - Attachment description: Bug 1538546 - Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACESS permission instead of the incognito manifest property. r?kmag!,mixedpuppy → Bug 1538546 - Check AddonManager PERM_CAN_CHANGE_PRIVATEBROWSING_ACCESS permission instead of the incognito manifest property. r?kmag!,mixedpuppy
Assignee: nobody → lgreco
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
Pushed by scaraveo@mozilla.com:
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 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
Attachment #9053290 - Flags: approval-mozilla-beta?
Flags: qe-verify?

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
Attachment #9053291 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

Flags: qe-verify? → qe-verify+
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

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.

Attachment #9053291 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9053290 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Flags: qe-verify+

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.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.