Closed Bug 1485953 Opened 1 year ago Closed 1 year ago

'Blocked Temporarily' permission is displayed in Control Center, if autoplay is configured to permanently block

Categories

(Firefox :: Site Identity, defect, P1)

63 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox63 --- verified

People

(Reporter: gyula.palko, Assigned: daleharvey)

References

Details

Attachments

(1 file, 1 obsolete file)

[Environment:]
Nightly 63.0a1 BuildId 20180823220048

[Steps:]
1. Open about:preferences#privacy
2. Select "Don't Autoplay" from autoplay dropdown menu
3. Open https://edition.cnn.com/videos in new tab(or any other site which triggers the autoplay doorhanger)
4. Open the Control Center and inspect the Permissions section


[Actual Result:]
 "Blocked Temporarily" is displayed for autoplay with sound

[Expected Result:]
 "Blocked" permission should be displayed, since autoplay is configured to permanently block
Assignee: nobody → dharvey
Attachment #9004064 - Flags: review?(jhofmann)
Attachment #9004064 - Flags: review?(francesco.lodolo)
Where does "Blocked Globally" come from? I've only seen "Blocked" for these cases, "globally" is a word never used across all Mozilla products.
Comment on attachment 9004064 [details] [diff] [review]
0001-Bug-1485953-Fix-label-for-permissions-blocked-global.patch

Review of attachment 9004064 [details] [diff] [review]:
-----------------------------------------------------------------

Huh, I actually thought I had checked this in my review, I must have misread the condition in getCurrentStateLabel...

::: browser/locales/en-US/chrome/browser/sitePermissions.properties
@@ +14,4 @@
>  state.current.allowedForSession = Allowed for Session
>  state.current.allowedTemporarily = Allowed Temporarily
>  state.current.blockedTemporarily = Blocked Temporarily
> +state.current.blockedTemporarily = Blocked Globally

I agree with flod that "Globally" is not a term we want to use here, just "Blocked" without an option to remove should be enough.

Also, the string ID still says "blockedTemporarily".

::: browser/modules/SitePermissions.jsm
@@ +687,1 @@
>          if (scope && scope != this.SCOPE_PERSISTENT && scope != this.SCOPE_POLICY)

It should be enough to just add a check for scope != this.SCOPE_GLOBAL here
Attachment #9004064 - Flags: review?(jhofmann) → review-
Priority: -- → P1
Should have checked the code around that, much simpler thanks
Attachment #9004064 - Attachment is obsolete: true
Attachment #9004064 - Flags: review?(francesco.lodolo)
Attachment #9004153 - Flags: review?(jhofmann)
Comment on attachment 9004153 [details] [diff] [review]
0001-Bug-1485953-Fix-label-for-permissions-blocked-global.patch

Review of attachment 9004153 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks!
Attachment #9004153 - Flags: review?(jhofmann) → review+
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14926b250441
Fix label for permissions blocked globally. r=johannh
https://hg.mozilla.org/mozilla-central/rev/14926b250441
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Verified, that the issue is no longer reproducible on Nightly 63.0a1(20180828220157).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.