Add test to confirm that video control show controls in different sizes correctly

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ralin, Assigned: ralin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Add a UI test to confirm video control has correct layout under different sizes.

1. controls in controlbar should show/hide according to width of video region.
2. controls in controlbar should display size properly. The size of button should be fixed, and flexible slider should resize accordingly.
3. controlbar should hide when it can't not fit in <video>'s height.
(Assignee)

Comment 1

2 years ago
Hi jaws,

Do you know how to get right(computed) size of each controls in mochitest? i.e. playButton.clientWidth // should return 30;
I've tried some API(clientWidth, getComputedStyle, getBoundingClientRect, etc.) and also wrapped the elements by SpecialPowers, but always got 0 instead of right size. Does different type of test matter(I currently run in plain mochitest)? I suspect that is because content process could not probe anonymous content correctly.

Thanks
Flags: needinfo?(jaws)
We use clientWidth in videocontrols.xml. As long as we have a reference to the right element clientWidth should work. Did you make sure that the video controls had appeared first? They will need to be visible to get layout to apply the styles and size the elements.
Flags: needinfo?(jaws)
(Assignee)

Comment 3

2 years ago
Created attachment 8809264 [details]
0001-Bug-1311700-Add-a-test-to-confirm-the-correctness-of.patch
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8809264 - Attachment is obsolete: true
Attachment #8809264 - Attachment is patch: false
Comment on attachment 8812639 [details]
Bug 1311700 - Part 1. add a test to confirm the correctness of video control in different sizes.

https://reviewboard.mozilla.org/r/94280/#review94770

::: toolkit/content/tests/widgets/test_videocontrols_size.html:45
(Diff revision 1)
> +  const minControlBarWidth = 48;
> +  const minClickToPlaySize = 48;
> +  const maxVolumeSliderWidth = 64;
> +
> +  function getElementName(elem) {
> +    return  elem.getAttribute("anonid") || elem.getAttribute("class");

nit, one space between 'return' and 'elem.getAttribute'

::: toolkit/content/tests/widgets/test_videocontrols_size.html:167
(Diff revision 1)
> +  function getElementWithinVideoByAttribute(video, aName, aValue) {
> +    const videoControl = domUtils.getChildrenForNode(video, true)[1];
> +
> +    return SpecialPowers.wrap(videoControl.ownerDocument)
> +      .getAnonymousElementByAttribute(videoControl, aName, aValue);
> +  }

This function should be moved to head.js and then removed from the other tests that also define it.
Attachment #8812639 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Blocks: 1319301
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bdb32806a818
Part 1. add a test to confirm the correctness of video control in different sizes. r=jaws
https://hg.mozilla.org/integration/autoland/rev/fd9319f6ddce
Part 2. move shared function to head.js. r=jaws
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bdb32806a818
https://hg.mozilla.org/mozilla-central/rev/fd9319f6ddce
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.