Closed Bug 1477294 Opened 6 years ago Closed 6 years ago

The permissions doorhanger is no longer dismissed when the page is refreshed right after clearing the media autoplay permission

Categories

(Firefox :: Site Identity, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- verified

People

(Reporter: cmuresan, Assigned: daleharvey)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

[Affected versions]:
- Nightly v63.0a1, Build ID 20180720101508

[Affected Platforms]:
- All Windows
- All Mac
- All Linux

[Steps to reproduce]:
1. Open the browser with a clean new profile.
2. Navigate to www.youtube.com and click a video thumbnail.
3. Click the "Allow Autoplay" button from the doorhanger.
4. Click the circled "i" button from the left part of the address bar.
5. Clear the "Play media with sound" permission.
6. Press F5 and observe the behavior.

[Expected results]:
- The doorhanger is dismissed while the page is refreshed. The media autoplay doorhanger appears again. 

[Actual results]:
- The page is refreshed, but the doorhanged is not dismissed and an additional media autoplay doorhanger is displayed over the previous one. (in some cases the first doorhanger will also appear cutoff)

[Regression window]:
 5:13.25 INFO: Last good revision: 0bce1e5b252a300d14ee22b408f76574c01553cd
 5:13.25 INFO: First bad revision: 8dcdd881fd09a220150f2931fda326b01996627e
 5:13.26 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0bce1e5b252a300d14ee22b408f76574c01553cd&tochange=8dcdd881fd09a220150f2931fda326b01996627e

[Notes]:
- The issue is also reproducible if you block the autoplay permission and then clear it.
- Attached a screen recording of the issue.
Dale, Chris, this one is probably of interest to one of you.
Flags: needinfo?(dharvey)
Flags: needinfo?(cpearce)
Priority: -- → P2
Blocks: block-autoplay
No longer blocks: 1476853
Flags: needinfo?(cpearce)
So I can reproduce this with any permission (https://harmonious-wombat.glitch.me/ does a geolocation on load), I dont believe any of the autoplay code will have affected this. Johannh do you know whats up here?
Flags: needinfo?(dharvey) → needinfo?(jhofmann)
Yeah I think that's just a panel issue, nothing too dramatic though...

Do you still want to have this block bug 1376321?
Component: Audio/Video: Playback → Site Identity and Permission Panels
Flags: needinfo?(jhofmann)
Priority: P2 → P3
Product: Core → Firefox
(In reply to Johann Hofmann [:johannh] - PTO July 24th - 31st from comment #3)
> Yeah I think that's just a panel issue, nothing too dramatic though...
> 
> Do you still want to have this block bug 1376321?

I think we should try to fix this before shipping block autoplay. Lots of sites use media playback, so the permissions panel will be getting used a lot.
Should this remain P3 if it's blocking shipping?
Flags: needinfo?(cpearce)
I'll go ahead and call this P2, will see if johannf disagrees....
Flags: needinfo?(cpearce)
Priority: P3 → P2
(In reply to Chris Pearce (:cpearce) from comment #6)
> I'll go ahead and call this P2, will see if johannf disagrees....

I meant johann*h*!
Johann: Are you able to take this bug? We think that the control center should disappear when the page is refreshed.
Flags: needinfo?(jhofmann)
I can't commit to fixing this in the 63 cycle. Prathiksha may be interested in taking it, though. Or anyone from your team, see below for instructions:

Note that we do not want to hide the control center on any reload, just user initiated ones. In this case that just means when the user used the shortcut, since we hide the control center on mouse interaction outside anyway.

This is done in these two locations, I'm not sure if the shortcut actually uses both of them, though (that would have to be looked into):

https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/browser/base/content/browser.js#3208
https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/browser/base/content/tabbrowser.js#3314
Flags: needinfo?(jhofmann)
cpearce -- have you found someone to take this, since soft freeze is next week?
Flags: needinfo?(cpearce)
I will see if I can get a patch up tomorrow
Assignee: nobody → dharvey
Flags: needinfo?(cpearce)
Attachment #9001684 - Flags: review?(jhofmann)
Comment on attachment 9001684 [details] [diff] [review]
0001-Bug-477294-Hide-identity-popup-when-the-user-reloads.patch

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

I don't think test really covers what you're changing here, but I don't think it blocks review either. Please take another look at the test, though.

Thanks!

::: browser/base/content/browser.js
@@ +3225,5 @@
>    // Reset temporary permissions on the current tab. This is done here
>    // because we only want to reset permissions on user reload.
>    SitePermissions.clearTemporaryPermissions(gBrowser.selectedBrowser);
> +  PanelMultiView.hidePopup(gIdentityHandler._identityPopup);
> +

nit: does this need two newlines below it?

::: browser/base/content/tabbrowser.js
@@ +3353,4 @@
>      // Reset temporary permissions on the current tab. This is done here
>      // because we only want to reset permissions on user reload.
>      SitePermissions.clearTemporaryPermissions(browser);
> +    PanelMultiView.hidePopup(gIdentityHandler._identityPopup);

nit: please add a newline below ;)

::: browser/base/content/test/permissions/browser_permissions.js
@@ +277,5 @@
> +
> +    let reloadButton = document.getElementById("reload-button");
> +    let reloaded = BrowserTestUtils.browserLoaded(browser, false, PERMISSIONS_PAGE);
> +
> +    EventUtils.synthesizeMouseAtCenter(reloadButton, {});

As I mentioned in the bug, this should have worked already, since we hide the control center when the user clicks anywhere outside. Ideally we could synthesize a refresh key (F5?) and make sure that it also hides the popup, since that's what this bug is about.
Attachment #9001684 - Flags: review?(jhofmann) → review+
Switched the test to do an f5 reload, cheers
Attachment #9001684 - Attachment is obsolete: true
Attachment #9002188 - Flags: review+
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/936a20fefe03
Hide identity popup when the user reloads. r=johannh
https://hg.mozilla.org/mozilla-central/rev/936a20fefe03
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Verified, that the issue is no longer reproducible on Nightly 63.0a1(20180819221915).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.