Bug 133268 – [Mac] media controls should prevent 'click' events from reaching the page

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

The C++ based media control elements stopped 'click' event propagation, the JS based controls should too.

[Eric Carlson]

<rdar://problem/16968818>

[Eric Carlson]

Created attachment 232103 [details]
Proposed patch.

[Xabier Rodríguez Calvar]

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.

[Jer Noble]

Comment on attachment 232103 [details]
Proposed patch.

Nice! r=me.

[Eric Carlson]

Created attachment 232136 [details]
Patch for landing.

[Eric Carlson]

(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.

[WebKit Commit Bot]

Comment on attachment 232136 [details]
Patch for landing.

Clearing flags on attachment: 232136

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

[Ahmad Saleem]

Landed - https://github.com/WebKit/WebKit/commit/4eb4f42ad145ae36a6597826cafc479a72200d73