Media controls behave strangely when changing media sources

• Component: Media
• Status: RESOLVED FIXED

[Wenson Hsieh]

Media controls behave strangely when changing media sources

[Wenson Hsieh]

<rdar://problem/28227805>

[Wenson Hsieh]

Created attachment 288710 [details]
Patch

[Wenson Hsieh]

Created attachment 288712 [details]
Rebased against trunk

[Tim Horton]

Comment on attachment 288712 [details]
Rebased against trunk

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

Looks pretty reasonable, but maybe have Jer or someone take a peek?

> Source/WebCore/ChangeLog:26
> +          play. This gives the user a change to interact with the controls when a video ends, but also allows the page

s/change/chance/

> Source/WebCore/html/HTMLMediaElement.cpp:166
> +static const double HideMediaControlsAfterEndedDelay = 6;

no std::chrono love? I don't know if we're on or off it this week, but it looks like you're matching nearby code so I guess it's fine.

> Source/WebCore/html/MediaElementSession.cpp:654
> +    return elementRectInMainFrame.width() * elementRectInMainFrame.height() > totalElementArea / 2;

Don't we have a .area()?

[Wenson Hsieh]

Comment on attachment 288712 [details]
Rebased against trunk

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

Thanks! I'll get Jer's opinion as well.

>> Source/WebCore/ChangeLog:26
>> +          play. This gives the user a change to interact with the controls when a video ends, but also allows the page
> 
> s/change/chance/

Whoops -- fixed.

>> Source/WebCore/html/MediaElementSession.cpp:654
>> +    return elementRectInMainFrame.width() * elementRectInMainFrame.height() > totalElementArea / 2;
> 
> Don't we have a .area()?

Ah, so we do! Changed to area().

[Jer Noble]

Comment on attachment 288712 [details]
Rebased against trunk

This all looks reasonable to me as well.

[Wenson Hsieh]

Created attachment 288881 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 288881 [details]
Patch for landing

Rejecting attachment 288881 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 288881, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/2075052

[Wenson Hsieh]

Created attachment 288882 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 288882 [details]
Patch for landing

Clearing flags on attachment: 288882

Committed r205938: <http://trac.webkit.org/changeset/205938>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.