Bug 117621 – [Mac] clicking caption track glyph should dismiss menu

• Component: Media
• Status: RESOLVED FIXED

[Antoine Quint]

Clicking on the caption glyph shows the caption menu if it is not visible. Clicking on it when the caption menu is already visible does nothing, it should dismiss the menu.

[Antoine Quint]

<rdar://problem/14143083>

[Antoine Quint]

Created attachment 204662 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 204662 [details]
Patch

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

> Source/WebCore/html/shadow/MediaControlsApple.cpp:600
>      if (event->type() == eventNames().mousewheelEvent && event->hasInterface(eventNames().interfaceForWheelEvent)) {

Nit (not your code, but): This if can be an else if since if the top case is true, this case can never be true. Or early return here and no need to else if.

[Eric Carlson]

Comment on attachment 204662 [details]
Patch

r=me, although it would be nice to address Joe's nit before landing.

[Antoine Quint]

Created attachment 204711 [details]
Patch for landing

[Antoine Quint]

(In reply to comment #3)
> (From update of attachment 204662 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204662&action=review
> 
> > Source/WebCore/html/shadow/MediaControlsApple.cpp:600
> >      if (event->type() == eventNames().mousewheelEvent && event->hasInterface(eventNames().interfaceForWheelEvent)) {
> 
> Nit (not your code, but): This if can be an else if since if the top case is true, this case can never be true. Or early return here and no need to else if.

Actually, very much my code from a previous patch, fixed in patch for landing.

[WebKit Commit Bot]

Comment on attachment 204711 [details]
Patch for landing

Clearing flags on attachment: 204711

Committed r151592: <http://trac.webkit.org/changeset/151592>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.