Bug 117621 - [Mac] clicking caption track glyph should dismiss menu
Summary: [Mac] clicking caption track glyph should dismiss menu
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-06-13 18:52 PDT by Antoine Quint
Modified: 2013-06-14 07:43 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.19 KB, patch)
2013-06-13 19:09 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (6.46 KB, patch)
2013-06-14 07:16 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2013-06-13 18:52:36 PDT
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.
Comment 1 Antoine Quint 2013-06-13 18:52:46 PDT
<rdar://problem/14143083>
Comment 2 Antoine Quint 2013-06-13 19:09:10 PDT
Created attachment 204662 [details]
Patch
Comment 3 Joseph Pecoraro 2013-06-13 23:47:19 PDT
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.
Comment 4 Eric Carlson 2013-06-14 06:44:51 PDT
Comment on attachment 204662 [details]
Patch

r=me, although it would be nice to address Joe's nit before landing.
Comment 5 Antoine Quint 2013-06-14 07:16:37 PDT
Created attachment 204711 [details]
Patch for landing
Comment 6 Antoine Quint 2013-06-14 07:21:17 PDT
(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.
Comment 7 WebKit Commit Bot 2013-06-14 07:43:13 PDT
Comment on attachment 204711 [details]
Patch for landing

Clearing flags on attachment: 204711

Committed r151592: <http://trac.webkit.org/changeset/151592>
Comment 8 WebKit Commit Bot 2013-06-14 07:43:15 PDT
All reviewed patches have been landed.  Closing bug.