Add test to confirm that fullscreen should(not) be available accordingly when video in an iframe

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ralin, Assigned: ralin)

Tracking

(Blocks 1 bug)

unspecified
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Fullscreen should be unavailable by default when video in an iframe. However, if iframe has `allowfullscreen` attribute should be available then.
Comment hidden (mozreview-request)

Comment 2

3 years ago
mozreview-review
Comment on attachment 8803764 [details]
Bug 1312324 - Add test to confirm fullscreen availability when video in an iframe.

https://reviewboard.mozilla.org/r/87904/#review87008

::: toolkit/content/tests/widgets/test_videocontrols_iframe_fullscreen.html:43
(Diff revision 1)
> +    })).then(() => new Promise(resolve => {
> +      const videoControl = domUtils.getChildrenForNode(video, true)[1];
> +      const controlBar = video.ownerDocument.getAnonymousElementByAttribute(
> +        videoControl, "class", "controlBar");
> +
> +      is(controlBar.getAttribute("fullscreen-unavailable") == "true", !available);

is() has a third argument which is a text explanation of what you're verifying here, please add "The controlbar should have an attribute marking whether fullscreen is available that corresponds to if the iframe has the allowfullscreen attribute."

Also, can you file a follow-up bug to change the attribute name to "fullscreen-available" and then reverse all of this logic? We should make sure that attributes and variable names of booleans don't include a negation in them.
Attachment #8803764 - Flags: review?(jaws) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 4

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b996ae995fe
Add test to confirm fullscreen availability when video in an iframe. r=jaws
Keywords: checkin-needed

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1b996ae995fe
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.