| Summary: | [mediacontrols] Test the ordering of elements in the controls panel | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||
| Component: | New Bugs | Assignee: | Dean Jackson <dino> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Dean Jackson
2015-09-04 10:25:25 PDT
Created attachment 260598 [details]
Patch
Comment on attachment 260598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260598&action=review > LayoutTests/media/controls/controls-test-helpers.js:40 > + var state = this.cachedCurrentState || this.currentState; Do you want "|" instead of "||" here? > LayoutTests/media/controls/elementOrder.html:19 > + Tiny Nit: this blank line isn't necessary. > LayoutTests/media/controls/elementOrder.html:33 > + elementNames.forEach(function (name) { > + var elementState = tester.stateForControlsElement(name); > + var leftValue = elementState.bounds.left; > + if (previousElementName && previousLeftValue) { > + tester.test(`${name} is to the right of ${previousElementName}`) > + .value(elementState.bounds.left) > + .isGreaterThan(previousLeftValue); > + } > + previousElementName = name; > + previousLeftValue = leftValue; > + }); Very nice! Comment on attachment 260598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260598&action=review >> LayoutTests/media/controls/controls-test-helpers.js:40 >> + var state = this.cachedCurrentState || this.currentState; > > Do you want "|" instead of "||" here? No, I want ||. If there is no cachedCurrentState, I want to get the new currentState. It looks a bit weird because this.currentState is a getter function (not just a value). Committed r189359: <http://trac.webkit.org/changeset/189359> |