Bug 133268 - [Mac] media controls should prevent 'click' events from reaching the page
Summary: [Mac] media controls should prevent 'click' events from reaching the page
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks: 133402
  Show dependency tree / graph
 
Reported: 2014-05-25 16:07 PDT by Eric Carlson
Modified: 2024-02-24 15:30 PST (History)
8 users (show)

See Also:

Attachments
Proposed patch. (9.36 KB, patch)
2014-05-26 20:27 PDT, Eric Carlson 		jer.noble: review+
Details | Formatted Diff | Diff
Patch for landing. (10.43 KB, patch)
2014-05-27 09:55 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>