Bug 148804 - [mediacontrols] Test the ordering of elements in the controls panel
Summary: [mediacontrols] Test the ordering of elements in the controls panel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-04 10:25 PDT by Dean Jackson
Modified: 2015-09-04 11:13 PDT (History)
1 user (show)

See Also:


Attachments
Patch (8.72 KB, patch)
2015-09-04 10:55 PDT, Dean Jackson
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2015-09-04 10:25:25 PDT
[mediacontrols] Test the ordering of elements in the controls panel
Comment 1 Radar WebKit Bug Importer 2015-09-04 10:28:44 PDT
<rdar://problem/22579232>
Comment 2 Dean Jackson 2015-09-04 10:55:55 PDT
Created attachment 260598 [details]
Patch
Comment 3 Eric Carlson 2015-09-04 11:01:27 PDT
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 4 Dean Jackson 2015-09-04 11:05:50 PDT
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).
Comment 5 Dean Jackson 2015-09-04 11:13:03 PDT
Committed r189359: <http://trac.webkit.org/changeset/189359>