Bug 170048 – AX: Media controls should be able to be re-activated after faded away

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Aaron Chu]

Once the media controls is faded away, they are impossible to make reappear using VO.

[Aaron Chu]

<rdar://problem/30157179>

[Aaron Chu]

Created attachment 305309 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Antoine Quint]

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.

[Aaron Chu]

Created attachment 305416 [details]
Patch

[Antoine Quint]

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?

[Build Bot]

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

[Build Bot]

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

[Aaron Chu]

Created attachment 305492 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 305492 [details]
Patch

Clearing flags on attachment: 305492

Committed r214469: <http://trac.webkit.org/changeset/214469>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.