Bug 131908 - [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks consist of unsupported track types
Summary: [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks consist of...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-19 22:17 PDT by Brent Fulgham
Modified: 2014-04-20 21:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.65 KB, patch)
2014-04-19 22:21 PDT, Brent Fulgham
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-04-19 22:17:48 PDT
When Webkit is asked to display a media track menu with out-of-band tracks of an unsupported track type (e.g., TTML), you are not able to select the 'Off' menu item.

There is no user-visible impact here (since the track doesn't display whether it is selected or not), but it's unexpected behavior to select the 'Off' menu element, but for the menu to not show an 'Off' checked state.
Comment 1 Brent Fulgham 2014-04-19 22:21:22 PDT
Created attachment 229756 [details]
Patch
Comment 2 Brent Fulgham 2014-04-19 22:21:59 PDT
Comment on attachment 229756 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:3717
> +    } else if (trackToSelect == TextTrack::captionMenuOffItem()) {

I'm not sure if this would be better to just incorporate into the loop above?
Comment 3 Eric Carlson 2014-04-20 20:23:29 PDT
Comment on attachment 229756 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        (WebCore::HTMLMediaElement::parseAttribute): When the user selects 'Off', make sure the currently 'showing'

prepare-ChangeLog doesn't like something about HTMLMediaElement.cpp so it thinks all changes are in parseAttribute, so you should fix this entry manually.

>> Source/WebCore/html/HTMLMediaElement.cpp:3717
>> +    } else if (trackToSelect == TextTrack::captionMenuOffItem()) {
> 
> I'm not sure if this would be better to just incorporate into the loop above?

It could be, but I think it is probably easier to follow the logic this way.

> Source/WebCore/html/HTMLMediaElement.cpp:3720
> +            if (!trackToSelect || track != trackToSelect)

trackToSelect is TextTrack::captionMenuOffItem so it can't be NULL, and TextTrack::captionMenuOffItem won't be in textTracks(), so this check can be removed.
Comment 4 Brent Fulgham 2014-04-20 21:28:59 PDT
Committed r167578: <http://trac.webkit.org/changeset/167578>