Bug 144926 - Controls should drop off when video gets smaller
Summary: Controls should drop off when video gets smaller
Status: RESOLVED DUPLICATE of bug 144973
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-12 12:49 PDT by Roger Fong
Modified: 2015-05-17 10:30 PDT (History)
6 users (show)

See Also:


Attachments
patch (25.73 KB, patch)
2015-05-13 01:04 PDT, Roger Fong
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-mavericks (710.99 KB, application/zip)
2015-05-13 01:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (686.05 KB, application/zip)
2015-05-13 01:43 PDT, Build Bot
no flags Details
patch (26.58 KB, patch)
2015-05-13 10:53 PDT, Roger Fong
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2015-05-12 12:49:40 PDT
<rdar://problem/20306227>

Controls should drop off in a predefined order as the video gets narrower.
Comment 1 Roger Fong 2015-05-13 01:04:16 PDT
Created attachment 253023 [details]
patch
Comment 2 Build Bot 2015-05-13 01:23:17 PDT
Comment on attachment 253023 [details]
patch

Attachment 253023 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4840607851741184

New failing tests:
media/controls-dropoff.html
fullscreen/video-controls-timeline.html
media/controls-drag-timebar.html
Comment 3 Build Bot 2015-05-13 01:23:19 PDT
Created attachment 253024 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 4 Build Bot 2015-05-13 01:43:48 PDT
Comment on attachment 253023 [details]
patch

Attachment 253023 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4690977633599488

New failing tests:
media/controls-dropoff.html
fullscreen/video-controls-timeline.html
media/controls-drag-timebar.html
Comment 5 Build Bot 2015-05-13 01:43:50 PDT
Created attachment 253025 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 6 Roger Fong 2015-05-13 10:53:28 PDT
Created attachment 253036 [details]
patch
Comment 7 Roger Fong 2015-05-14 14:27:32 PDT

*** This bug has been marked as a duplicate of bug 144973 ***
Comment 8 Darin Adler 2015-05-17 10:30:58 PDT
Comment on attachment 253036 [details]
patch

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

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:52
> +Controller.PlayButtonWidth = 44;
> +Controller.RewindButtonWidth = 32;
> +Controller.MuteButtonWidth = 30;
> +Controller.WirelessTargetPickerButtonWidth = 32;
> +Controller.CaptionButtonWidth = 32;
> +Controller.FullScreenButtonWidth = 30;
> +Controller.MinimumTimelineWidth = 200;

It seems really inflexible to hard-wire all these widths. Is there any alternative?

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:626
>              this.controls.statusDisplay.innerText = this.UIString('Error');
> -        else if (this.isLive && this.video.readyState >= HTMLMediaElement.HAVE_CURRENT_DATA)
> +            this.minimumStatusWidth = 50;

It looks like the width here of 50 is based on the width of the string; what if the localized version is wider?

By the way, I think we should use textContent instead of innerText because I believe it is more efficient.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:629
>              this.controls.statusDisplay.innerText = this.UIString('Live Broadcast');
> -        else if (this.video.networkState === HTMLMediaElement.NETWORK_LOADING)
> +            this.minimumStatusWidth = 100;

It looks like the width here of 100 is based on the width of the string; what if the localized version is wider?

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:632
>              this.controls.statusDisplay.innerText = this.UIString('Loading');
> -        else
> +            this.minimumStatusWidth = 75;

It looks like the width here of 75 is based on the width of the string; what if the localized version is wider?

> Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm:369
> -        CGContextSetFontAntialiasingStyle(cgContext, kCGFontAntialiasingStyleUnfilteredCustomDilation);
> +        CGContextSetFontAntialiasingStyle(cgContext, kCGFontAntialiasingStyleFilteredCustomDilation);

Please don’t land this change as part of this patch!