[webkit-reviews] review granted: [Bug 131908] [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks consist of unsupported track types : [Attachment 229756] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 131908: [Mac] Unable to select 'Off' or 'Auto' from track menu when tracks
consist of unsupported track types
https://bugs.webkit.org/show_bug.cgi?id=131908

Attachment 229756: Patch
https://bugs.webkit.org/attachment.cgi?id=229756&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
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.