Bug 170048 - AX: Media controls should be able to be re-activated after faded away
Summary: AX: Media controls should be able to be re-activated after faded away
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Safari 10
Hardware: All All
: P2 Normal
Assignee: Aaron Chu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-03-24 01:27 PDT by Aaron Chu
Modified: 2017-03-28 00:50 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.32 KB, patch)
2017-03-24 11:44 PDT, Aaron Chu
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-elcapitan (1.97 MB, application/zip)
2017-03-24 16:58 PDT, Build Bot
no flags Details
Patch (4.78 KB, patch)
2017-03-26 00:22 PDT, Aaron Chu
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (867.14 KB, application/zip)
2017-03-27 01:12 PDT, Build Bot
no flags Details
Patch (4.75 KB, patch)
2017-03-27 12:05 PDT, Aaron Chu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Chu 2017-03-24 01:27:04 PDT
Once the media controls is faded away, they are impossible to make reappear using VO.
Comment 1 Aaron Chu 2017-03-24 01:27:40 PDT
<rdar://problem/30157179>
Comment 2 Aaron Chu 2017-03-24 11:44:53 PDT
Created attachment 305309 [details]
Patch
Comment 3 Build Bot 2017-03-24 16:58:45 PDT
Comment on attachment 305309 [details]
Patch

Attachment 305309 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3406155

New failing tests:
http/tests/preload/download_resources.html
Comment 4 Build Bot 2017-03-24 16:58:48 PDT
Created attachment 305342 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Antoine Quint 2017-03-25 10:24:48 PDT
Comment on attachment 305309 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305309&action=review

Not a fan of this approach. The layout delegate is concerned with layout, and this is setting a property on it that is focus-related, so I don't think that it works. Also, I don't think it's a good approach to set a property on a delegate, typically you'd have a delegation pattern where an IconButton informs its delegate that it has received or lost focus and maintaining a focused state is then the responsibility of the delegate.

But couldn't we take advantage of event bubbling here and register "focusin" and "focusout" event listeners on the MediaControls instance? This would require no change to IconButton and also would allow other types of focusable items, such as the volume slider and scrubbers to be tracked.

> Source/WebCore/Modules/modern-media-controls/controls/icon-button.js:88
> +        else if (event.type === "focus" && event.currentTarget === this.element)

WebKit style is closing curly bracket and else statement on the same line like it was before.
Comment 6 Aaron Chu 2017-03-26 00:22:32 PDT
Created attachment 305416 [details]
Patch
Comment 7 Antoine Quint 2017-03-26 11:03:29 PDT
Comment on attachment 305416 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305416&action=review

> Source/WebCore/Modules/modern-media-controls/controls/controls-bar.js:172
> +            } else if (event.type === "focus" || event.type === "focusin")

Do we still need to listen to and handle the "focus" event at all?
Comment 8 Build Bot 2017-03-27 01:12:45 PDT
Comment on attachment 305416 [details]
Patch

Attachment 305416 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3416118

New failing tests:
fast/history/page-cache-createObjectURL-using-open-panel.html
Comment 9 Build Bot 2017-03-27 01:12:47 PDT
Created attachment 305455 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 10 Aaron Chu 2017-03-27 12:05:58 PDT
Created attachment 305492 [details]
Patch
Comment 11 WebKit Commit Bot 2017-03-28 00:49:59 PDT
Comment on attachment 305492 [details]
Patch

Clearing flags on attachment: 305492

Committed r214469: <http://trac.webkit.org/changeset/214469>
Comment 12 WebKit Commit Bot 2017-03-28 00:50:03 PDT
All reviewed patches have been landed.  Closing bug.