Summary: | [GTK] Missing WebPreferences for media playback requiring user gestures and inline playback | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Pena <spenap> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mrobinson, pnormand, spenap, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Simon Pena
2012-04-30 07:48:44 PDT
Created attachment 139466 [details]
Patch
(In reply to comment #1) > Created an attachment (id=139466) [details] > Patch I'm not sure if this patch deserves new tests: in bug #64742, no tests were added, and they were similar changes. I noticed an almost-flaky test (media/track/track-cues-pause-on-exit.html). It's been failing sometimes (timing out) after applying this test, but not always. I don't think it is related to this change, but anyway. Comment on attachment 139466 [details]
Patch
Alright, looks good to me but we'd need another reviewer to +1. Will you provide a similar patch but for WebKit2?
(In reply to comment #3) > (From update of attachment 139466 [details]) > Alright, looks good to me but we'd need another reviewer to +1. Will you provide a similar patch but for WebKit2? Yes: I'll report a new bug with a patch for the WebKit2 part tomorrow. Martin can you please have a look at this patch? I did a first review but we need another +1 for the added API. Thanks! Comment on attachment 139466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139466&action=review Looks good. r- only because you don't have commit access yet the documentation isn't quite clear. > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:985 > + * Whether an user gesture would be required to start media playback or load > + * media. This is off by default, so media playback could start > + * automatically. Setting it on requires a gesture by the user to start > + * playback, or to load the media. > + * Weird English rules exception nit: Whether an user -> Whether a user I think this documentation could be slightly more clear about what kind of gesture is required to start playback. For instance, if I turn this on, what will I need to do to trigger playback? Mouse over the page? Mouse over the element itself? Click on the element? (In reply to comment #6) > (From update of attachment 139466 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139466&action=review > > Looks good. r- only because you don't have commit access yet the documentation isn't quite clear. > > > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:985 > > + * Whether an user gesture would be required to start media playback or load > > + * media. This is off by default, so media playback could start > > + * automatically. Setting it on requires a gesture by the user to start > > + * playback, or to load the media. > > + * > > Weird English rules exception nit: Whether an user -> Whether a user Thanks for catching, I'll fix this. > I think this documentation could be slightly more clear about what kind of gesture is required to start playback. For instance, if I turn this on, what will I need to do to trigger playback? Mouse over the page? Mouse over the element itself? Click on the element? From « LayoutTests/media/video-play-require-user-gesture.html », I understand it typically involves clicking on the Play button, but probably other gestures could be required. I guess I can make the documentation read something like: « Whether a user gesture (typically clicking on the play button) would be required to start media playback or load media... » What do you think? I'll use the same description for the WK2 patch. Sounds good to me! Created attachment 140951 [details]
Patch
Comment on attachment 140951 [details] Patch Clearing flags on attachment: 140951 Committed r116541: <http://trac.webkit.org/changeset/116541> All reviewed patches have been landed. Closing bug. |