Closed Bug 1357995 Opened 7 years ago Closed 7 years ago

Permaorange test_trigger_fullscreen_by_pointer_events.html | uncaught exception - TypeError: target.requestFullscreen is not a function when Gecko 55 merges to beta on 2017-06-12

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Version:
55 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 + fixed

People

(Reporter: philor, Assigned: stone)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1357995 - Run the full screen test in a new window so that we can access to fullscreen API after pushing preference.
59 bytes, text/x-review-board-request
masayuki
: review+
Details 
You do indeed seem to be setting the right prefs in https://dxr.mozilla.org/mozilla-central/source/dom/events/test/pointerevents/test_trigger_fullscreen_by_pointer_events.html?q=file%3Atest_trigger_fullscreen_by_pointer_events.html#42 but according to https://treeherder.mozilla.org/logviewer.html#?job_id=92882780&repo=try (which you should be able to reproduce locally by just changing /config/milestone.txt from 55a1 to 55) they aren't going to actually work on beta.

[Tracking Requested - why for this release]:

Merge bustage, closed tree, delayed b1, sadness.
Flags: needinfo?(sshih)
Oops. I should open a new window and push the permission of requestFullscreen. It didn't fail on nightly because the preference is 'on' on nightly [1]

[1] http://searchfox.org/mozilla-central/rev/d8ac097a1de2544f0e51d186bc850662c5b7430a/modules/libpref/init/all.js#4827
Assignee: nobody → sshih
Flags: needinfo?(sshih)
(In reply to Phil Ringnalda (:philor) from comment #0)
> You do indeed seem to be setting the right prefs in
> https://dxr.mozilla.org/mozilla-central/source/dom/events/test/pointerevents/
> test_trigger_fullscreen_by_pointer_events.
> html?q=file%3Atest_trigger_fullscreen_by_pointer_events.html#42 but
> according to
> https://treeherder.mozilla.org/logviewer.html#?job_id=92882780&repo=try
> (which you should be able to reproduce locally by just changing
> /config/milestone.txt from 55a1 to 55) they aren't going to actually work on
> beta.
> 
> [Tracking Requested - why for this release]:
> 
> Merge bustage, closed tree, delayed b1, sadness.
Sorry for causing this problem. I created a patch to fix the bad test case. What I could do at this moment to prevent blocking the merge?
Tracking 55+ for this permaorange, especially to avoid delays in B1.
tracking-firefox55: ? → +
(In reply to Ming-Chou Shih [:stone] from comment #2)
> Sorry for causing this problem. I created a patch to fix the bad test case.
> What I could do at this moment to prevent blocking the merge?

There's no hurry, this is bustage that will happen in June if it's not fixed, not currently happening bustage, you still have 7 weeks.
Comment on attachment 8860255 [details]
Bug 1357995 - Run the full screen test in a new window so that we can access to fullscreen API after pushing preference.

https://reviewboard.mozilla.org/r/132262/#review135148

::: dom/events/test/pointerevents/test_trigger_fullscreen_by_pointer_events.html:15
(Diff revision 1)
>  <script>
> -
>  SimpleTest.waitForExplicitFinish();
>  
> -var target = document.getElementById("target");
> +function startTest() {
> +  let win = window.open("data:text/html,<body><div id='target' style='width: 50px; height: 50px; background: green'></div></body>", "_blank");

Although, I'm not familiar with fullscreen API. So, I don't understand why opening new window can fix this bug...

::: dom/events/test/pointerevents/test_trigger_fullscreen_by_pointer_events.html:17
(Diff revision 1)
>  SimpleTest.waitForExplicitFinish();
>  
> -var target = document.getElementById("target");
> +function startTest() {
> +  let win = window.open("data:text/html,<body><div id='target' style='width: 50px; height: 50px; background: green'></div></body>", "_blank");
> +  win.addEventListener("load", (e) => {
> +  let target = win.document.getElementById("target");

nit: one more indent level needed here.

::: dom/events/test/pointerevents/test_trigger_fullscreen_by_pointer_events.html:36
(Diff revision 1)
> -
> -function startTest() {
> +    SpecialPowers.pushPrefEnv({
> +      "set": [
> +        ["full-screen-api.unprefix.enabled", true],
> +        ["full-screen-api.allow-trusted-requests-only", false],
> +        ["dom.w3c_pointer_events.enabled", true]
> +      ]
> +    }, () => {
> -  // synthesize mouse events to generate pointer events and enter full screen.
> +      // synthesize mouse events to generate pointer events and enter full screen.
> -  synthesizeMouseAtCenter(target, { type: "mousedown" });
> -  synthesizeMouseAtCenter(target, { type: "mouseup" });
> +      synthesizeMouseAtCenter(target, { type: "mousedown" }, win);
> +      synthesizeMouseAtCenter(target, { type: "mouseup" }, win);
> +    });
> +  });
>  }
>  
>  SimpleTest.waitForFocus(() => {
>    SpecialPowers.pushPrefEnv({
>      "set": [
>        ["full-screen-api.unprefix.enabled", true],
>        ["full-screen-api.allow-trusted-requests-only", false],
>        ["dom.w3c_pointer_events.enabled", true]
>      ]
>    }, startTest);

Do we need to set same prefs twice? Why?

If this is really necessary, please add comment the reason.
Attachment #8860255 - Flags: review?(masayuki) → review+
Attachment #8859884 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f7087d394f1e
Run the full screen test in a new window so that we can access to fullscreen API after pushing preference. r=masayuki
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f7087d394f1e
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: