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

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[Gyula Palko[:gpalko]]

[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

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: 0001-Bug-1485953-Fix-label-for-permissions-blocked-global.patch

Comment 2

[Francesco Lodolo [:flod]]

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 3

[Johann Hofmann [:johannh]]

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

Comment 4

[Dale Harvey (:daleharvey)]

Attachment: 0001-Bug-1485953-Fix-label-for-permissions-blocked-global.patch

Should have checked the code around that, much simpler thanks

Comment 5

[Johann Hofmann [:johannh]]

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!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14926b250441
Fix label for permissions blocked globally. r=johannh

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/14926b250441

Comment 8

[Gyula Palko[:gpalko]]

Verified, that the issue is no longer reproducible on Nightly 63.0a1(20180828220157).