Bug 159661 - REGRESSION (202694): Audio and Video playback controls: Cannot find a position slider to adjust playback position using VO.
Summary: REGRESSION (202694): Audio and Video playback controls: Cannot find a positio...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-07-11 20:21 PDT by Dean Jackson
Modified: 2016-07-12 16:40 PDT (History)
1 user (show)

See Also:

Attachments
Patch (5.05 KB, patch)
2016-07-11 20:27 PDT, Dean Jackson 		eric.carlson: review+
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 Dean Jackson 2016-07-11 20:21:53 PDT 
The change in r202694 broke the visibility of the scrubber in MediaDocuments.
Comment 1 Dean Jackson 2016-07-11 20:22:50 PDT 
<rdar://problem/27285135>
Comment 2 Dean Jackson 2016-07-11 20:27:51 PDT 
Created attachment 283384 [details]
Patch
Comment 3 Eric Carlson 2016-07-12 07:53:44 PDT 
Comment on attachment 283384 [details]
Patch

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

> LayoutTests/media/controls/default-size-should-show-scrubber.html:11
> +let tester = new ControlsTest()

Nit: this can be a const

> LayoutTests/media/controls/default-size-should-show-scrubber.html:37
> +        let elementState = tester.stateForControlsElement(name);

Ditto.
Comment 4 Eric Carlson 2016-07-12 11:07:26 PDT 
Comment on attachment 283384 [details]
Patch

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

> LayoutTests/media/controls/default-size-should-show-scrubber.html:6
> +    if (state === null || typeof state.className !== "string")
> +        return false;

Don't you want to return true here?

> LayoutTests/media/controls/default-size-should-show-scrubber.html:7
> +    let classes = state.className.split();

Nit: this can be a const.
Comment 5 Dean Jackson 2016-07-12 11:53:11 PDT 
Committed r203120: <http://trac.webkit.org/changeset/203120>
Comment 6 Ryan Haddad 2016-07-12 16:31:16 PDT 
The test added with this change is frequently failing on El Capitan Debug WK2

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fcontrols%2Fdefault-size-should-show-scrubber.html
Comment 7 Ryan Haddad 2016-07-12 16:40:24 PDT 
(In reply to comment #6)
> The test added with this change is frequently failing on El Capitan Debug WK2
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=media%2Fcontrols%2Fdefault-size-should-show-
> scrubber.html

Direct link to a failure:

https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/builds/6506