1251659 - [e10s] test_bug967796.html shouldn't use setBoolPref

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P4)

Description

[Andrew McCreight [:mccr8]]

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]

Comment 1

[Andrew McCreight [:mccr8]]

There's a bunch of tests in that directory that use setBoolPref or setIntPref. I'll fix them up.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: part 1 - test_bug967796.html should use SpecialPowers.pushPrefEnv.

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

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Remove redundant setting of pointer events pref in test_bug822898.

The same preference is set earlier using pushPrefEnv.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

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?

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b61ca87b91e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a116d8cb42f

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for making test_bug967796.html permafail across the board.
https://treeherder.mozilla.org/logviewer.html#?job_id=22593129&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/dfb1e32a7061

Comment 7

[Andrew McCreight [:mccr8]]

I thought I had tested it locally both with and without e10s, but looks like I only tested it with e10s.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8f6706615a64
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cd4ce001c7d

Comment 9

[Andrew McCreight [:mccr8]]

I backed out part 1 again. Maybe the new failures are intermittent. I see green on L32, but not L64.

Comment 10

[Andrew McCreight [:mccr8]]

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/10fe67d9c5be

Comment 11

[Martijn Wargers (dead)]

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.

Comment 12

[Andrew McCreight [:mccr8]]

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.

Comment 13

[Chris Peterson [:cpeterson]]

Andrew, are you still working on this test fix? How important is re-enabling this test for e10s?

Comment 14

[Andrew McCreight [:mccr8]]

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?

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

This is important but not before we have pointer events enabled. So, not yet.

Comment 16

[Chris Peterson [:cpeterson]]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Stone, is this something you could investigate by chance?

Comment 18

[Ming-Chou Shih [:stone]]

Attachment: part 1 - test_bug967796.html should use SpecialPowers.pushPrefEnv

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2dde2bec054ca9fa79774955369fbe92608e0697

Comment 19

[Ming-Chou Shih [:stone]]

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.

Comment 20

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7898d31656fabff263bbf95cff222cf7b5730fe
No intermittent failure of test_bug967796.html found

Comment 21

[Andrew McCreight [:mccr8]]

Thanks for finishing this up!

Comment 22

[Ming-Chou Shih [:stone]]

Attachment: test_bug967796.html should use SpecialPowers.pushPrefEnv. r=smaug

Updated the patch summary.

Comment 23

[Ming-Chou Shih [:stone]]

checkin-needed for attachment #8816395 [details] [diff] [review]. Thanks.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd30cdbf03c3
test_bug967796.html should use SpecialPowers.pushPrefEnv. r=smaug

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bd30cdbf03c3

Comment 26

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.