[e10s] test_bug967796.html shouldn't use setBoolPref

RESOLVED FIXED

Status

()

defect
P4
normal
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: mccr8, Assigned: stone)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox47 affected)

Details

(Whiteboard: btpp-active)

Attachments

(2 attachments, 2 obsolete attachments)

This apparently fails intermittently when enabled for e10s, though I'm not sure offhand what the failure looks like. It uses setBoolPref which seems like a reasonable possible cause for this.

The failures can be seen in the log linked here:
  https://bugzilla.mozilla.org/show_bug.cgi?id=1219842#c11

The errors look like this, which sounds like a pref not being set properly:
 08:00:48 INFO - 286 INFO TEST-UNEXPECTED-FAIL | dom/events/test/test_bug967796.html | Wrong related target (pointerenter) - got null, expected [object HTMLDivElement]
There's a bunch of tests in that directory that use setBoolPref or setIntPref. I'll fix them up.
Whiteboard: btpp-active
I retriggered a half dozen times and I didn't see the orange. Though maybe it is the wrong platform or something.

The various wheel event related tests in this directory have a ton of preferences, so I'll deal with them in a separate bug.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=511b9a1fd934
Attachment #8724462 - Flags: review?(mrbkap)
The same preference is set earlier using pushPrefEnv.
Attachment #8724463 - Flags: review?(mrbkap)
Comment on attachment 8724462 [details] [diff] [review]
part 1 - test_bug967796.html should use SpecialPowers.pushPrefEnv.

Review of attachment 8724462 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the comment addressed.

::: dom/events/test/test_bug967796.html
@@ +23,4 @@
>  SimpleTest.waitForExplicitFinish();
> +
> +SpecialPowers.pushPrefEnv({"set": [["dom.w3c_pointer_events.enabled", true]]},
> +                          SimpleTest.waitForFocus(runTests));

You probably meant to wrap the waitForFocus in a closure, right?
Attachment #8724462 - Flags: review?(mrbkap) → review+
Attachment #8724463 - Flags: review?(mrbkap) → review+
I thought I had tested it locally both with and without e10s, but looks like I only tested it with e10s.
I backed out part 1 again. Maybe the new failures are intermittent. I see green on L32, but not L64.
Keywords: leave-open
Blocks: e10s-tests
tracking-e10s: --- → +
Isn't it better to do:

SimpleTest.waitForFocus(function() {
SpecialPowers.pushPrefEnv({"set": [["dom.w3c_pointer_events.enabled", true]]},
runTests);
});
 
That seems more reliable to me. Because the focus event can happen every time, while pushPrefEnv would still be going on.
I actually did that in my initial landing in comment 5. :)

This test used to fail intermittently on non-e10s, in bug 1170449. I wonder if my change somehow is breaking this workaround.
See Also: → 1170449
Andrew, are you still working on this test fix? How important is re-enabling this test for e10s?
Flags: needinfo?(continuation)
I'm not working on this. I didn't get around to investigating why it was failing intermittently. Presumably some other e10s race condition.

Olli, how important do you think this test is?
Assignee: continuation → nobody
Flags: needinfo?(continuation) → needinfo?(bugs)
This is important but not before we have pointer events enabled. So, not yet.
Flags: needinfo?(bugs)
Thanks. I'll make this test bug depend on "enable pointer events" bug 1166347 and prioritize it to be revisited after the initial e10s release.
Depends on: 1166347
Priority: -- → P4
Stone, is this something you could investigate by chance?
Flags: needinfo?(sshih)
Assignee: nobody → sshih
Flags: needinfo?(sshih)
The major problem of the original test case is the iframe is created before setting preference 'dom.w3c_pointer_events.enabled' to true. Refined the test case as creating iframe dynamically after the preference is set.
Attachment #8803809 - Flags: review?(bugs)
Attachment #8803809 - Flags: review?(bugs) → review+
Thanks for finishing this up!
Updated the patch summary.
Attachment #8803809 - Attachment is obsolete: true
Attachment #8816395 - Flags: review+
checkin-needed for attachment #8816395 [details] [diff] [review]. Thanks.
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd30cdbf03c3
test_bug967796.html should use SpecialPowers.pushPrefEnv. r=smaug
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.