If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[e10s] Make test_wheel_default_action.html use pushPrefEnv

RESOLVED FIXED in Firefox 47

Status

()

Core
DOM: Events
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

(Blocks: 1 bug)

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(6 attachments)

(Assignee)

Description

2 years ago
This alone does not make the test work with e10s, but I feel like it should help prevent intermittent failures.
(Assignee)

Comment 1

2 years ago
Created attachment 8724464 [details] [diff] [review]
part 1 - Use pushPrefEnv in setDeltaMultiplierSettings in window_wheel_default_action.html.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=511b9a1fd934
Attachment #8724464 - Flags: review?(masayuki)
(Assignee)

Comment 2

2 years ago
Created attachment 8724465 [details] [diff] [review]
part 2 - Use pushPrefEnv in doTestActionOverride in window_wheel_default_action.html.
Attachment #8724465 - Flags: review?(masayuki)
(Assignee)

Comment 3

2 years ago
Created attachment 8724466 [details] [diff] [review]
part 3 - Use pushPrefEnv in doTestWholeScroll in window_wheel_default_action.html.
Attachment #8724466 - Flags: review?(masayuki)
(Assignee)

Comment 4

2 years ago
Created attachment 8724467 [details] [diff] [review]
part 4 - Use pushPrefEnv in doTestActionOverride in window_wheel_default_action.html.
Attachment #8724467 - Flags: review?(masayuki)
(Assignee)

Comment 5

2 years ago
Created attachment 8724468 [details] [diff] [review]
part 5 - Make prepare() in doTestScroll take a callback, and use pushPrefEnv.
Attachment #8724468 - Flags: review?(masayuki)
(Assignee)

Comment 6

2 years ago
Created attachment 8724469 [details] [diff] [review]
part 6 - Make cleanup() in doTestScroll take a callback, and use pushPrefEnv.
Attachment #8724469 - Flags: review?(masayuki)
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+
(Assignee)

Comment 9

2 years ago
(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+
(Assignee)

Comment 12

2 years ago
(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.

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4077e20d4cb9
https://hg.mozilla.org/integration/mozilla-inbound/rev/abf3fcbf174f
https://hg.mozilla.org/integration/mozilla-inbound/rev/77c018b57e8c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce44f62a57b9
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea8a0ba359a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ffc4e71ecb
Whiteboard: btpp-active
(Assignee)

Updated

2 years ago
Blocks: 1252251

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4077e20d4cb9
https://hg.mozilla.org/mozilla-central/rev/abf3fcbf174f
https://hg.mozilla.org/mozilla-central/rev/77c018b57e8c
https://hg.mozilla.org/mozilla-central/rev/ce44f62a57b9
https://hg.mozilla.org/mozilla-central/rev/ea8a0ba359a2
https://hg.mozilla.org/mozilla-central/rev/c7ffc4e71ecb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.