Bug 94176 - [BlackBerry] Some media controls are mispositioned for dynamic live streams (HLS)
Summary: [BlackBerry] Some media controls are mispositioned for dynamic live streams (...
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Feil
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-15 20:05 PDT by Max Feil
Modified: 2012-11-02 12:51 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.58 KB, patch)
2012-08-15 20:15 PDT, Max Feil
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Feil 2012-08-15 20:05:06 PDT
The default HTML5 media controls for dynamic live streams have no timeline or timeline container, which for BlackBerry results in mispositioning of the buttons that are supposed to be to the right of the timeline (fullscreen and mute). Instead of being right justified they incorrectly appear right next to the play button. The fix is to explicitly position these 2 buttons whenever the media duration is infinite (indicating a live stream).

An automated layout test is not possible for this patch because dynamic live streams require a special dedicated web server. Putting an external video URL into an automated test is not correct either. So I have created a manual test that points to an external HLS video that works today.
Comment 1 Max Feil 2012-08-15 20:15:36 PDT
Created attachment 158696 [details]
Patch
Comment 2 Antonio Gomes 2012-08-16 07:21:27 PDT
Comment on attachment 158696 [details]
Patch

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

r+

I have one comment. Feel free to address it or not. If you do, please reupload the patch with pre-filled "Reviewed by Antonio Gomes" in the changelog and commit message, and just request cq? Thanks.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:771
> +    Length zero(0, Fixed);

I would have defined "zero" within the if where it is used.
Comment 3 Max Feil 2012-08-16 12:58:03 PDT
Comment on attachment 158696 [details]
Patch

I think it's cleaner to declare "zero" with all the other Length values. That way they are all grouped together, and can be freely re-used anywhere in the function in the future.
Comment 4 WebKit Review Bot 2012-08-16 13:54:37 PDT
Comment on attachment 158696 [details]
Patch

Clearing flags on attachment: 158696

Committed r125811: <http://trac.webkit.org/changeset/125811>
Comment 5 WebKit Review Bot 2012-08-16 13:54:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Max Feil 2012-11-02 12:51:09 PDT
Closing bug for patch that landed a long time ago.