Closed Bug 1311700 Opened 7 years ago Closed 7 years ago

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


(Toolkit :: Video/Audio Controls, defect)

Not set



Tracking Status
firefox53 --- fixed


(Reporter: ralin, Assigned: ralin)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

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.
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.

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: nobody → ralin
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.

::: 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 on attachment 8813961 [details]
Bug 1311700 - Part 2. move shared function to head.js.
Attachment #8813961 - Flags: review?(jaws) → review+
Blocks: 1319301
Keywords: checkin-needed
Pushed by
Part 1. add a test to confirm the correctness of video control in different sizes. r=jaws
Part 2. move shared function to head.js. r=jaws
Keywords: checkin-needed
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.