Bug 172375

Summary: Inherit media user gestures from the top document when autoplay quirks are allowed
Product: WebKit Reporter: Matt Rajca <mrajca>
Component: MediaAssignee: Matt Rajca <mrajca>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, cdumez, dbates, eric.carlson, esprehn+autocc, jer.noble, kangil.han
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch eric.carlson: review+

Matt Rajca
Reported 2017-05-19 12:24:39 PDT
Inherit media user gesture from document when autoplay quirks are allowed.
Attachments
Patch (1.58 KB, patch)
2017-05-19 12:27 PDT, Matt Rajca 		no flags
DetailsFormatted DiffDiff
Patch (7.76 KB, patch)
2017-05-19 14:23 PDT, Matt Rajca 		no flags
DetailsFormatted DiffDiff
Patch (7.87 KB, patch)
2017-05-19 14:34 PDT, Matt Rajca 		eric.carlson: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Matt Rajca
Comment 1 2017-05-19 12:27:16 PDT
Created attachment 310683 [details] Patch
Alex Christensen
Comment 2 2017-05-19 12:31:15 PDT
Comment on attachment 310683 [details] Patch Can this be tested?
Matt Rajca
Comment 3 2017-05-19 12:33:58 PDT
(In reply to Alex Christensen from comment #2) > Comment on attachment 310683 [details] > Patch > > Can this be tested? We already have a test for 'mediaUserGestureInheritsFromDocument'. Not sure how valuable it is testing: auto* loader = this->loader(); if (loader && loader->allowsAutoplayQuirks())
Daniel Bates
Comment 4 2017-05-19 12:55:01 PDT
(In reply to Matt Rajca from comment #3) > (In reply to Alex Christensen from comment #2) > > Comment on attachment 310683 [details] > > Patch > > > > Can this be tested? > > We already have a test for 'mediaUserGestureInheritsFromDocument'. Not sure > how valuable it is testing: > > auto* loader = this->loader(); > if (loader && loader->allowsAutoplayQuirks()) It is valuable to test this code to ensure that we do not regress the behavior of this SPI. In general, every change that has an observable effect should include a test if it is feasible to write one.
Matt Rajca
Comment 5 2017-05-19 12:59:27 PDT
(In reply to Daniel Bates from comment #4) > (In reply to Matt Rajca from comment #3) > > (In reply to Alex Christensen from comment #2) > > > Comment on attachment 310683 [details] > > > Patch > > > > > > Can this be tested? > > > > We already have a test for 'mediaUserGestureInheritsFromDocument'. Not sure > > how valuable it is testing: > > > > auto* loader = this->loader(); > > if (loader && loader->allowsAutoplayQuirks()) > > It is valuable to test this code to ensure that we do not regress the > behavior of this SPI. In general, every change that has an observable effect > should include a test if it is feasible to write one. OK, will add one.
Matt Rajca
Comment 6 2017-05-19 14:23:48 PDT
Created attachment 310698 [details] Patch
Matt Rajca
Comment 7 2017-05-19 14:34:33 PDT
Created attachment 310702 [details] Patch
Matt Rajca
Comment 8 2017-05-19 14:57:22 PDT
r217152
Note You need to log in before you can comment on or make changes to this bug.