Closed Bug 1251837 Opened 8 years ago Closed 8 years ago

[e10s] Make test_wheel_default_action.html use pushPrefEnv

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

(Whiteboard: btpp-active)

Attachments

(6 files)

This alone does not make the test work with e10s, but I feel like it should help prevent intermittent failures.
Attachment #8724464 - Flags: review?(masayuki) → review+
Comment on attachment 8724465 [details] [diff] [review]
part 2 - Use pushPrefEnv in doTestActionOverride in window_wheel_default_action.html.

># HG changeset patch
># User Andrew McCreight <continuation@gmail.com>
>
>Bug 1251837, part 2 - Use pushPrefEnv in doTestActionOverride in window_wheel_default_action.html. r=masayuki
>
>diff --git a/dom/events/test/window_wheel_default_action.html b/dom/events/test/window_wheel_default_action.html
>index afa8e10..e1cdce5 100644
>--- a/dom/events/test/window_wheel_default_action.html
>+++ b/dom/events/test/window_wheel_default_action.html
>@@ -1744,25 +1744,29 @@ function doTestActionOverride(aCallback)
>       }
>     }, 20);
>   }
>   doIt();
> }
> 
> function runTests()
> {
>-  SpecialPowers.setBoolPref("general.smoothScroll", false);
>-
>-  SpecialPowers.setIntPref("mousewheel.default.action", 1);      // scroll
>-  SpecialPowers.setIntPref("mousewheel.default.action.override_x", -1);
>-  SpecialPowers.setIntPref("mousewheel.with_shift.action", 2);   // history
>-  SpecialPowers.setIntPref("mousewheel.with_shift.action.override_x", -1);
>-  SpecialPowers.setIntPref("mousewheel.with_control.action", 3); // zoom
>-  SpecialPowers.setIntPref("mousewheel.with_control.action.override_x", -1);
>+  SpecialPowers.pushPrefEnv({"set": [
>+    ["general.smoothScroll", false],
>+    ["mousewheel.default.action", 1],      // scroll
>+    ["mousewheel.default.action.override_x", -1],

You set them with pushPrefEnv here...

> function finishTests()
> {
>-  SpecialPowers.clearUserPref("general.smoothScroll");
>-
>   SpecialPowers.clearUserPref("mousewheel.default.action");
>   SpecialPowers.clearUserPref("mousewheel.default.action.override_x");

But you don't remove these lines from here, why? If this is a mistake, please fix it before landing. Otherwise (e.g., setting code is still there, keep current code).
Attachment #8724465 - Flags: review?(masayuki) → review+
Attachment #8724466 - Flags: review?(masayuki) → review+
Comment on attachment 8724467 [details] [diff] [review]
part 4 - Use pushPrefEnv in doTestActionOverride in window_wheel_default_action.html.

> function finishTests()
> {
>   SpecialPowers.clearUserPref("mousewheel.default.action");
>-  SpecialPowers.clearUserPref("mousewheel.default.action.override_x");

Okay, I understood. "mousewheel.default.action" must be still set without pushPrefEnv.
Attachment #8724467 - Flags: review?(masayuki) → review+
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #8)
> Okay, I understood. "mousewheel.default.action" must be still set without
> pushPrefEnv.

Yes, all of the clearUserPref calls are removed by the end of the patch queue. Sorry, I should have mentioned that. Thanks for the reviews.
Comment on attachment 8724468 [details] [diff] [review]
part 5 - Make prepare() in doTestScroll take a callback, and use pushPrefEnv.

nit: I don't like the name doNextTestCont. How about doTestCurrentScroll()? Because the caller is doTestScroll() and the function does perform a test with currentTestIndex.
Attachment #8724468 - Flags: review?(masayuki) → review+
Comment on attachment 8724469 [details] [diff] [review]
part 6 - Make cleanup() in doTestScroll take a callback, and use pushPrefEnv.

Thank you for your work for this complicated test! I'm really happy!!
Attachment #8724469 - Flags: review?(masayuki) → review+
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #10)
> nit: I don't like the name doNextTestCont. How about doTestCurrentScroll()?
> Because the caller is doTestScroll() and the function does perform a test
> with currentTestIndex.

That's a better name. I'll rename it.
Whiteboard: btpp-active
Blocks: 1252251
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: