Closed
Bug 1513681
Opened 5 years ago
Closed 5 years ago
Show block-autoplay-icon when site is blocked globally
Categories
(Firefox :: Site Identity, enhancement)
Firefox
Site Identity
Tracking
()
RESOLVED
FIXED
Firefox 66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: alwu, Assigned: alwu)
References
Details
Attachments
(6 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
As we're going to remove doorhanger in bug1513039, so the logic which was implemented in bug1476555 in order to show block-autoplay-icon won't work afterward. The present logic is that gecko send the `ContentPermissionRequest` which would causes prompting or showing the icon, but gecko won't send any autoplay request anymore after removing all doorhanger related codes. Therefore, we should find a new way to show the icon.
Assignee | ||
Comment 1•5 years ago
|
||
Hi, Dale, Could you give me any suggestion how to hook the block-icon to permission center without sending a request from gecko C++ side? Thank you!
Flags: needinfo?(dharvey)
Comment 2•5 years ago
|
||
Hey Alastor I dont know off the top of my head and it is very possible that the best way to implement it is the way it is already implemented. I gave a r+ to https://bugzilla.mozilla.org/show_bug.cgi?id=1513039 assuming there was already a plan for this, but if not then I would very much suggest not deleting all the current code, set set media.autoplay.default to BLOCK, handle PROMPT like BLOCK and remove the code paths that are only hit by PROMPT Then this is done, and if it can / needs to be refactored and cleaned up it can be
Flags: needinfo?(dharvey)
Assignee | ||
Comment 3•5 years ago
|
||
This event is used to notify tab that this site is permanently blocked and we should show the blocking icon for it. Patch2 will handle following details.
Assignee | ||
Comment 4•5 years ago
|
||
Handle the process from receiving event to showing the block icon. Therefore, 'AudibleAutoplayMediaOccurred' is used for autoplay shield study which has been finished, we can remove it.
Assignee | ||
Comment 5•5 years ago
|
||
Use more proper name for actor which will handle all autoplay related event.
Assignee | ||
Comment 6•5 years ago
|
||
We've handle showing the blocking icon in patch2, so we don't need to set block permission in PermissionUI.
Updated•5 years ago
|
Attachment #9032006 -
Attachment description: Bug 1513681 - part2 : handle 'GloballyAutoplayBlocked' event and remove unused event → Bug 1513681 - part2 : handle 'GloballyAutoplayBlocked' event
Assignee | ||
Comment 7•5 years ago
|
||
This event is used for shield-study which has finished, so we could remove it.
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → alwu
Assignee | ||
Updated•5 years ago
|
Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/23b5a58e3bcc part1 : dispatch 'GloballyAutoplayBlocked' event when site is permanent blocked. r=cpearce,smaug https://hg.mozilla.org/integration/autoland/rev/d0a9422928ae part2 : handle 'GloballyAutoplayBlocked' event r=jaws,daleharvey https://hg.mozilla.org/integration/autoland/rev/79a78732c3ac part3 : rename 'AudibleAutoplayChild' actor r=jaws https://hg.mozilla.org/integration/autoland/rev/6f52b229d953 part4 : remove the logic about setting globally blocked in PermissionUI. r=daleharvey https://hg.mozilla.org/integration/autoland/rev/d24ddb803761 part5 : remove event 'AudibleAutoplayMediaOccurred'. r=jaws
Comment 9•5 years ago
|
||
Backed out 5 changesets (Bug 1513681) for browser_autoplay_blocked.js failures Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d24ddb803761f36c05030455c1144bcce9f2a5b2&selectedJob=219169828 Backout link: https://hg.mozilla.org/integration/autoland/rev/0c02d1f0db78ff9e5b2be24f8f40bb6383c17f4e Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=219169828&repo=autoland&lineNumber=2018 17:40:08 INFO - Buffered messages finished 17:40:08 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_autoplay_blocked.js | Blocked icon is shown - 17:40:08 INFO - Stack trace: 17:40:08 INFO - chrome://mochikit/content/browser-test.js:test_ok:1305 17:40:08 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible/<:45 17:40:08 INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111 17:40:08 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible:41 17:40:08 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1106 17:40:08 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1097 17:40:08 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995 17:40:08 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803 17:40:08 INFO - GECKO(1988) | [Parent 1988, Main Thread] WARNING: '!mPresContext', file /builds/worker/workspace/build/src/dom/events/UIEvent.cpp, line 165 17:40:08 INFO - GECKO(1988) | [Parent 1988, Main Thread] WARNING: '!mPresContext', file /builds/worker/workspace/build/src/dom/events/UIEvent.cpp, line 179 17:40:08 INFO - Console message: [JavaScript Warning: "Autoplay is only allowed when approved by the user, the site is activated by the user, or media is muted." {file: "https://example.com/browser/browser/base/content/test/permissions/browser_autoplay_blocked.html" line: 0}] 17:40:09 INFO - Not taking screenshot here: see the one that was previously logged 17:40:09 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_autoplay_blocked.js | List of permissions is not empty - 17:40:09 INFO - Stack trace: 17:40:09 INFO - chrome://mochikit/content/browser-test.js:test_ok:1305 17:40:09 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible/<:48 17:40:09 INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111 17:40:09 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible:41 17:40:09 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1106 17:40:09 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1097 17:40:09 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995 17:40:09 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803 17:40:09 INFO - Not taking screenshot here: see the one that was previously logged 17:40:09 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_autoplay_blocked.js | One permission visible in main view - Got 0, expected 1 17:40:09 INFO - Stack trace: 17:40:09 INFO - chrome://mochikit/content/browser-test.js:test_is:1316 17:40:09 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible/<:51 17:40:09 INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111 17:40:09 INFO - chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:testMainViewVisible:41 17:40:09 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1106 17:40:09 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1097 17:40:09 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995 17:40:09 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803 17:40:09 INFO - Not taking screenshot here: see the one that was previously logged 17:40:09 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_autoplay_blocked.js | Uncaught exception - at chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:52 - TypeError: labels[0] is undefined 17:40:09 INFO - Stack trace: 17:40:09 INFO - testMainViewVisible/<@chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:52:5 17:40:09 INFO - async*withNewTab@resource://testing-common/BrowserTestUtils.jsm:111:24 17:40:09 INFO - async*testMainViewVisible@chrome://mochitests/content/browser/browser/base/content/test/permissions/browser_autoplay_blocked.js:41:9 17:40:09 INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1106:34 17:40:09 INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1097:16 17:40:09 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:995:9 17:40:09 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59 17:40:09 INFO - Leaving test bound testMainViewVisible 17:40:09 INFO - GECKO(1988) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration. 17:40:09 INFO - GECKO(1988) | MEMORY STAT | vsize 4358MB | residentFast 329MB | heapAllocated 129MB 17:40:09 INFO - TEST-OK | browser/base/content/test/permissions/browser_autoplay_blocked.js | took 3840ms 17:40:09 INFO - Not taking screenshot here: see the one that was previously logged 17:40:09 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_autoplay_blocked.js | Found an unexpected tab at the end of test run: https://example.com/browser/browser/base/content/test/permissions/browser_autoplay_blocked.html - 17:40:09 INFO - GECKO(1988) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /var/folders/wh/8l3t57c95fb4p5qxbmr_y80r00000w/T/tmp7H_XW7.mozrunner/runtests_leaks_tab_pid2002.log 17:40:09 INFO - checking window state
Flags: needinfo?(alwu)
Updated•5 years ago
|
Attachment #9032005 -
Attachment description: Bug 1513681 - part1 : dispatch 'GloballyAutoplayBlocked' event when site is permanent blocked. → Bug 1513681 - part1 : dispatch 'GloballyAutoplayBlocked' event when site is blocked
Assignee | ||
Comment 10•5 years ago
|
||
To check icon after tab receives `GloballyAutoplayBlocked` event.
Assignee | ||
Comment 11•5 years ago
|
||
Modify test in new patch, here is try-result. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1bfaf7fd3997d7f4cef2206ec8fd86f9dd0f8317
Flags: needinfo?(alwu)
Assignee | ||
Comment 12•5 years ago
|
||
Hi, Dale, Do you mind helping me review the patch6? Thank you!
Flags: needinfo?(dharvey)
Comment 13•5 years ago
|
||
Will do, sorry about the delay, having trouble distinguishing phabricator emails
Flags: needinfo?(dharvey)
Assignee | ||
Comment 14•5 years ago
|
||
Push to Try server [1] again after rebasing. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf4f15752e63181b5b01e02af66b566775940aa1
Comment 15•5 years ago
|
||
Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9cb5b5f77ed8 part1 : dispatch 'GloballyAutoplayBlocked' event when site is blocked r=cpearce,smaug https://hg.mozilla.org/integration/autoland/rev/d99f4d17ea92 part2 : handle 'GloballyAutoplayBlocked' event r=jaws,daleharvey https://hg.mozilla.org/integration/autoland/rev/239ede916b0d part3 : rename 'AudibleAutoplayChild' actor r=jaws https://hg.mozilla.org/integration/autoland/rev/9378c3b5ad2d part4 : remove the logic about setting globally blocked in PermissionUI. r=daleharvey https://hg.mozilla.org/integration/autoland/rev/86afca3f41ac part5 : remove event 'AudibleAutoplayMediaOccurred'. r=jaws https://hg.mozilla.org/integration/autoland/rev/fedf648d3785 part6 : modify test. r=daleharvey
Comment 16•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9cb5b5f77ed8 https://hg.mozilla.org/mozilla-central/rev/d99f4d17ea92 https://hg.mozilla.org/mozilla-central/rev/239ede916b0d https://hg.mozilla.org/mozilla-central/rev/9378c3b5ad2d https://hg.mozilla.org/mozilla-central/rev/86afca3f41ac https://hg.mozilla.org/mozilla-central/rev/fedf648d3785
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in
before you can comment on or make changes to this bug.
Description
•