Closed Bug 1538583 Opened 1 year ago Closed 1 year ago

Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox67 --- verified
firefox68 --- verified
firefox69 --- verified

People

(Reporter: kmag, Assigned: trishul.goel)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Static themes are always allowed in private windows, and cannot run any code which should make that a concern. Showing this loud warning for all of them serves no purpose, and is decidedly ugly.

If this is confirmed, can I work on this?

(In reply to Trishul from comment #1)

If this is confirmed, can I work on this?

Sure. There's nothing to confirm. Note that it will probably need a test, though.

Assignee: nobody → trishul.goel
Status: NEW → ASSIGNED

This should probably be done on top of D24720 in bug 1538546, or maybe even D24990 in bug 1536459 if it touches the same test code. And if we can reproduce this in beta, we'll want to uplift it.

Depends on: 1538546
Priority: -- → P1

Thanks Shane, I am on it now.

Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice

Pushed by scaraveo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/92cfc564dbff
Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice r=kmag

Comment on attachment 9054503 [details]
Bug 1538583: Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1380809
  • User impact if declined: In about:addons list view, loud purple badges on themes when they are not applicable to themes.
  • 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: load some themes and verify there are not purple badges
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Very simple fix. Thanks Trishul!
  • String changes made/needed: none
Attachment #9054503 - Flags: approval-mozilla-beta?
Flags: qe-verify?
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Just landed, I would like to see it verified on Nightly before we uplift it to Beta, thanks.

Flags: qe-verify? → qe-verify+
Attached image Bug1538583.gif

I tested on Firefox 68.0a1 (20190331215222) and on 68.0a1 (20190323094805) under Win 7 64-bit and the bug is not reproducing on the fixed build, also I can’t reproduce the bug on the affected build.

Could you please give me some STR to see how this bug is reproducing on the affected build?

Thanks!

Flags: needinfo?(trishul.goel)
Whiteboard: [qa-triaged]

Trishul, could you get back to Cosmin please? (see comment #10) Thanks.

Hello everyone, apologies for late reply.
For this specific bug, I didn't have STRs from UI, I had to use flags in my code to reproduce it and write tests accordingly.
Maybe Kris can help us with this.

Flags: needinfo?(trishul.goel) → needinfo?(kmaglione+bmo)
QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]

You should just need to install a static WebExtension theme and look at the themes section of about:addons

Flags: needinfo?(kmaglione+bmo)

:Trishul, could you do a followup patch where you add the same check in Extension.jsm? That is the actual point where the builtin themes are gaining the permission.

https://searchfox.org/mozilla-central/rev/9ee63566281365f26e7a4b06c9d4e2219e64c3e8/toolkit/components/extensions/Extension.jsm#1932

QE: you wont be able to reproduce because a patch on nightly that kmag landed caused the issue, and the patch here, which has landed, hides the issue.

This is not reproducible on beta, but my request to Trishul above should go onto beta to prevent giving the permission to non-extensions on upgrade. If you undo the patch here, you'll be able to see the badge for any builtin theme that has been enabled.

Flags: needinfo?(trishul.goel)

patch on nightly that kmag landed caused the issue

exposed the issue.

Sure, on it.

Flags: needinfo?(trishul.goel)
Attached file Bug 1538583 (obsolete) —

Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice

Comment on attachment 9054503 [details]
Bug 1538583: Themes should not show [ALLOWED IN PRIVATE WINDOWS] notice

Uplift approved for 67 beta 10, thanks.

Attachment #9054503 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attached image Bug1538583.png

This issue is verified as fixed on Firefox 69.0a1 (20190522152821), Firefox 68.0b3 (20190521110747) and Firefox 67.0 (20190516215225) under Win 7 64-bit and Mac OS X 10.14.1.

There are no purple badges displayed for the installed themes.

Please see the attached screenshot.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Trishul, was D26298 needed? If so, update it and get it landed, otherwise abandon the change.

Flags: needinfo?(trishul.goel)

Hi Shane, I believe its an overkill and is already handled by the previous patch D25411, so I will abandon this change.

Flags: needinfo?(trishul.goel)
Attachment #9056102 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.