Closed Bug 1428722 Opened 7 years ago Closed 7 years ago

Autoplay permission in not transfered correctly from a child frame to a different domain top frame

Categories

(Core :: Audio/Video: Playback, defect, P3)

Product:
Core
Component:
Audio/Video: Playback
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- verified

People

(Reporter: alwu, Assigned: alwu)

References

(
URL
)

Details

(Whiteboard: [block-ap-v1])

Attachments

(3 files)

Bug 1428722 - part1 : always activate the top level frame.
59 bytes, text/x-review-board-request
smaug
: review+
Details
Bug 1428722 - part2 : move all user-activation implementation details to nsDocument.
59 bytes, text/x-review-board-request
smaug
: review+
Details
Bug 1428722 - part3 : add tests.
59 bytes, text/x-review-board-request
smaug
: review+
Details
STR. 1. goto https://alastor0325.github.io/htmltests/autoplay_tests/autoplay_test4.html 2. click inside the right frame (updog) Expect. 3. Video from main frame and from left iframe (github.io) should start playing Actual. 3. Only video from right frame (updog) starts playing
Priority: -- → P3
Priority: P3 → --
Whiteboard: block-ap-v1
Priority: -- → P3
Whiteboard: block-ap-v1 → block-apv1
Whiteboard: block-apv1 → block-ap-v1
Attachment #8940665 - Flags: review?(bugs)
Attachment #8941783 - Flags: review?(bugs)
Attachment #8941784 - Flags: review?(bugs)
(In reply to Alastor Wu [:alwu][please needinfo me][GMT+8] from comment #0) > STR. > 1. goto > https://alastor0325.github.io/htmltests/autoplay_tests/autoplay_test4.html > 2. click inside the right frame (updog) > > Expect. > 3. Video from main frame and from left iframe (github.io) should start > playing > > Actual. > 3. Only video from right frame (updog) starts playing All the videos play here.
Comment on attachment 8940665 [details] Bug 1428722 - part1 : always activate the top level frame. https://reviewboard.mozilla.org/r/210912/#review218242 I don't understand the logic here. If there is top level page from domain A, it has iframe (A') from domain A as well, and that one has an iframe from domain B, and B is activated, we don't activate A' but we do activate top level page and the iframe for B. Why such behavior? In general I don't know what behavior we want, but if we want to activate the top level page, we should probably also activate all its same-origin descendant browsing contexts, no?
Attachment #8940665 - Flags: review?(bugs) → review-
Comment on attachment 8941783 [details] Bug 1428722 - part2 : move all user-activation implementation details to nsDocument. https://reviewboard.mozilla.org/r/212032/#review218244 This may need some changes once the first patch is updated, but looks reasonable for testing.
Attachment #8941783 - Flags: review?(bugs) → review+
oh, I see, HasBeenUserActivated() does the ancestor check.
Comment on attachment 8940665 [details] Bug 1428722 - part1 : always activate the top level frame. https://reviewboard.mozilla.org/r/210912/#review218248
Attachment #8940665 - Flags: review- → review+
Comment on attachment 8941784 [details] Bug 1428722 - part3 : add tests. https://reviewboard.mozilla.org/r/212034/#review218254 ::: toolkit/content/tests/browser/browser.ini:39 (Diff revision 2) > +support-files = > + file_autoplay_three_layers_frame1.html > + file_autoplay_three_layers_frame2.html > + file_autoplay_two_layers_frame1.html > + file_autoplay_two_layers_frame2.html > + file_video.html I don't see file_video.html anywhere in the tree. Is this patch on top of some other patches? ::: toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:39 (Diff revision 2) > +} > + > +var frameTestArray = [ > + { name: "A1_A2", layersNum: 2, src: PAGE_A1_A2 }, > + { name: "A1_B2", layersNum: 2, src: PAGE_A1_B2 }, > + { name: "A1_B2_C3", layersNum: 3, src: PAGE_A1_B2_C3 }, Since I don't know what file_video.html does, I don't know whether this actually tests domain C. ::: toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:55 (Diff revision 2) > + let tab = await BrowserTestUtils.openNewForegroundTab(window.gBrowser, > + "about:blank"); > + tab.linkedBrowser.loadURI(testSrc); > + await BrowserTestUtils.browserLoaded(tab.linkedBrowser); > + > + // If frame doesn't be activated, the video would play fail. "If the frame isn't activated, the video play will fail." Or something like that. ::: toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:88 (Diff revision 2) > + doc.notifyUserActivation(); > + } > + await ContentTask.spawn(tab.linkedBrowser, [layerIdx, testName], > + activate_frame); > + > + // If frame is activated, the video will success to play. will succeed ::: toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:125 (Diff revision 2) > + shouldFail = layerIdx >= 2 && !isActiveLayer; > + break; > + case "A1_B2_A3": > + shouldSuccess = layerIdx != 2 || > + (layerIdx == 2 && isActiveLayer); > + shouldFail = layerIdx == 2; This is weird. shouldSuccess is true when layerIdx == 2 and isActiveLayer is true and shouldFail is true when layerIdx is true then too. Shouldn't shouldFail be something like layerIdx == 2 && !isActiveLayer Please fix.
Attachment #8941784 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #14) > I don't see file_video.html anywhere in the tree. Is this patch on top of > some other patches? Ah, sorry, "file_video.html" was introduced in bug1424113. > ::: > toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:55 > (Diff revision 2) > > + let tab = await BrowserTestUtils.openNewForegroundTab(window.gBrowser, > > + "about:blank"); > > + tab.linkedBrowser.loadURI(testSrc); > > + await BrowserTestUtils.browserLoaded(tab.linkedBrowser); > > + > > + // If frame doesn't be activated, the video would play fail. > > "If the frame isn't activated, the video play will fail." > > Or something like that. OK. > ::: > toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:88 > (Diff revision 2) > > + doc.notifyUserActivation(); > > + } > > + await ContentTask.spawn(tab.linkedBrowser, [layerIdx, testName], > > + activate_frame); > > + > > + // If frame is activated, the video will success to play. > > will succeed > OK. > ::: > toolkit/content/tests/browser/browser_autoplay_policy_iframe_hierarchy.js:125 > (Diff revision 2) > > + shouldFail = layerIdx >= 2 && !isActiveLayer; > > + break; > > + case "A1_B2_A3": > > + shouldSuccess = layerIdx != 2 || > > + (layerIdx == 2 && isActiveLayer); > > + shouldFail = layerIdx == 2; > > This is weird. shouldSuccess is true when layerIdx == 2 and isActiveLayer is > true and > shouldFail is true when layerIdx is true then too. > Shouldn't shouldFail be something like > layerIdx == 2 && !isActiveLayer > > Please fix. OK.
Pushed by bjones@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b9056b64d45b part1 : always activate the top level frame. r=smaug https://hg.mozilla.org/integration/autoland/rev/adbad272045d part2 : move all user-activation implementation details to nsDocument. r=smaug https://hg.mozilla.org/integration/autoland/rev/6f9b763bb1c9 part3 : add tests. r=smaug
Whiteboard: block-ap-v1 → [block-ap-v1]
Flags: qe-verify+
I managed to reproduce the initial issue on 59.0a1 (2018-01-08) using the STR from comment 0 and the following prefs: - media.autoplay.enabled set to false - media.autoplay.enabled.user-gestures-needed set to true I can confirm that 59.0b6 build1 (20180201171410) is verified fixed across platforms (Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13.2).
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: