Bug 144973 – [MediaControls] Refactor media controls & bring improvements made to iOS controls to Mac.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MediaControls] Refactor media controls & bring improvements made to iOS controls to Mac.

[Jer Noble]

Created attachment 253071 [details]
Patch

[Jer Noble]

Comment on attachment 253071 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:3788
> +    RefPtr<HTMLMediaElement> strongThis = this;
> +    std::function<void()> task = [strongThis] {
> +        if (ShadowRoot* root = strongThis->userAgentShadowRoot())
> +            root->dispatchEvent(Event::create("resize", false, false));
> +    };
> +    document().postTask(task);

This should use a GenericTaskQueue so that it gets cancelled if the HTMLMediaElement is destroyed; i'll change this in a future patch.

[Roger Fong]

*** Bug 144926 has been marked as a duplicate of this bug. ***

[Roger Fong]

Created attachment 253145 [details]
helper patch

This patch makes it so that the button positions are defined using both left and right margins, instead of just one or the other.
This makes it so that each button is essentially defined as the button and the 8pixels to the left and right of it, which allows the control dropoff algorithm to not have to use a spacer element to take the place of the timeline when the video gets too small.

Without this change, the volume and play buttons could potentially appear to be right next to each other at some video widths.

[Dean Jackson]

Comment on attachment 253145 [details]
helper patch

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

> Source/WebCore/ChangeLog:9
> +        The only visual change here is the swapping of the rewind and play button positions.

Technically we should be able to do that using the flex order. This is ok for now, but I'd like to get the OS X controls to use the order property for the arrangement of the buttons.

[Jer Noble]

Committed r184359: <http://trac.webkit.org/changeset/184359>

[Roger Fong]

reopened until Jer's patch lands.

[WebKit Commit Bot]

Comment on attachment 253145 [details]
helper patch

Clearing flags on attachment: 253145

Committed r184361: <http://trac.webkit.org/changeset/184361>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 145035

[Jer Noble]

I believe the crashes that led to the patch's rollout were due to a pre-existing bug in RenderFlowThread. I filed bug #145042 which should address the underlying cause of the crashes, and if so, I'll roll this patch back in.

[Jer Noble]

(In reply to comment #10)
> I believe the crashes that led to the patch's rollout were due to a
> pre-existing bug in RenderFlowThread. I filed bug #145042 which should
> address the underlying cause of the crashes, and if so, I'll roll this patch
> back in.

And with the patch from that bug in place, I no longer see the crashes in the region test with /this/ patch in place.

[Jer Noble]

Created attachment 253218 [details]
Patch for landing.

[Jer Noble]

Comment on attachment 253218 [details]
Patch for landing.

EWS seems happy, and all tests succeed locally. Marking as cq+.

[WebKit Commit Bot]

Comment on attachment 253218 [details]
Patch for landing.

Clearing flags on attachment: 253218

Committed r184416: <http://trac.webkit.org/changeset/184416>