Bug 133268

Summary: [Mac] media controls should prevent 'click' events from reaching the page
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, calvaris, commit-queue, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 133402    
Attachments:
Description Flags
Proposed patch.
jer.noble: review+
Patch for landing. none

Description Eric Carlson 2014-05-25 16:07:18 PDT 
The C++ based media control elements stopped 'click' event propagation, the JS based controls should too.
Comment 1 Eric Carlson 2014-05-25 16:08:01 PDT 
<rdar://problem/16968818>
Comment 2 Eric Carlson 2014-05-26 20:27:09 PDT 
Created attachment 232103 [details]
Proposed patch.
Comment 3 Xabier Rodríguez Calvar 2014-05-27 01:03:09 PDT 
Comment on attachment 232103 [details]
Proposed patch.

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

We are only testing the playback button, though the bug is about all elements and not only play button. It could be interesting to try more elements than the playback button.

Another interesting thing would be adding the return clauses to the mediaControlsGtk.js. I could do it myself but it doesn't seem like a very complicated and dangerous task.

> LayoutTests/media/media-controls-cancel-events.html:35
> +                waitForEvent("playing", endTest);

We could add a a test failure for errors so that the test doesn't time out in case playback fails for whatever reason. It is not very important though.
Comment 4 Jer Noble 2014-05-27 08:35:36 PDT 
Comment on attachment 232103 [details]
Proposed patch.

Nice! r=me.
Comment 5 Eric Carlson 2014-05-27 09:55:28 PDT 
Created attachment 232136 [details]
Patch for landing.
Comment 6 Eric Carlson 2014-05-27 09:56:15 PDT 
(In reply to comment #3)
> (From update of attachment 232103 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232103&action=review
> 
> We are only testing the playback button, though the bug is about all elements and not only play button. It could be interesting to try more elements than the playback button.
> 

  Good point, done.

> Another interesting thing would be adding the return clauses to the mediaControlsGtk.js. I could do it myself but it doesn't seem like a very complicated and dangerous task.
> 
> > LayoutTests/media/media-controls-cancel-events.html:35
> > +                waitForEvent("playing", endTest);
> 

  I won't be able to test changes to the GTK controls so I would prefer if someone else made those changes.
Comment 7 WebKit Commit Bot 2014-05-27 10:33:38 PDT 
Comment on attachment 232136 [details]
Patch for landing.

Clearing flags on attachment: 232136

Committed r169387: <http://trac.webkit.org/changeset/169387>