From cad8f463fc8cef1f3b9079355975c8a78ff6023b Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Tue, 6 Aug 2024 14:33:54 +0000 Subject: [PATCH] Bug 1741469 part 3: Add an off-by-default pref to control whether input type="number"/"range" fields are modified when mousewheel-scrolled. a=dmeehan The C++ code change here is simply replacing an ifdef with a pref-check (and associated reindentation for being placed in an "if"-block). The pref is off-by-default for now, since no other browser seems to modify these fields through mousewheel scrolling, and users seem to be surprised when they encounter the behavior in Firefox. See discussion on the bugzilla page. Note: now that this behavior is controlled by a pref, we don't need to have platform-specific #ifdefs and "skip-if" test exemptions. If we decide to bring this behavior back on certain platforms, we can do so by simply changing the default value of the pref on those platforms. Original Revision: https://phabricator.services.mozilla.com/D175719 Differential Revision: https://phabricator.services.mozilla.com/D217523 --- dom/html/HTMLInputElement.cpp | 53 +++++++++++++----------- dom/html/test/mochitest.toml | 14 ------- dom/html/test/test_bug1261673.html | 25 +++++++++-- dom/html/test/test_bug1261674-1.html | 25 +++++++++-- modules/libpref/init/StaticPrefList.yaml | 8 ++++ 5 files changed, 78 insertions(+), 47 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index ecca0b40e986..859de24a467a 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -3944,39 +3944,42 @@ nsresult HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor) { } break; } -#if !defined(ANDROID) && !defined(XP_MACOSX) case eWheel: { - // Handle wheel events as increasing / decreasing the input element's - // value when it's focused and it's type is number or range. - WidgetWheelEvent* wheelEvent = aVisitor.mEvent->AsWheelEvent(); - if (!aVisitor.mEvent->DefaultPrevented() && - aVisitor.mEvent->IsTrusted() && IsMutable() && wheelEvent && - wheelEvent->mDeltaY != 0 && - wheelEvent->mDeltaMode != WheelEvent_Binding::DOM_DELTA_PIXEL) { - if (mType == FormControlType::InputNumber) { - if (nsContentUtils::IsFocusedContent(this)) { - StepNumberControlForUserEvent(wheelEvent->mDeltaY > 0 ? -1 : 1); + if (StaticPrefs:: + dom_input_number_and_range_modified_by_mousewheel()) { + // Handle wheel events as increasing / decreasing the input + // element's value when it's focused and it's type is number or + // range. + WidgetWheelEvent* wheelEvent = aVisitor.mEvent->AsWheelEvent(); + if (!aVisitor.mEvent->DefaultPrevented() && + aVisitor.mEvent->IsTrusted() && IsMutable() && wheelEvent && + wheelEvent->mDeltaY != 0 && + wheelEvent->mDeltaMode != WheelEvent_Binding::DOM_DELTA_PIXEL) { + if (mType == FormControlType::InputNumber) { + if (nsContentUtils::IsFocusedContent(this)) { + StepNumberControlForUserEvent(wheelEvent->mDeltaY > 0 ? -1 + : 1); + FireChangeEventIfNeeded(); + aVisitor.mEvent->PreventDefault(); + } + } else if (mType == FormControlType::InputRange && + nsContentUtils::IsFocusedContent(this) && + GetMinimum() < GetMaximum()) { + Decimal value = GetValueAsDecimal(); + Decimal step = GetStep(); + if (step == kStepAny) { + step = GetDefaultStep(); + } + MOZ_ASSERT(value.isFinite() && step.isFinite()); + SetValueOfRangeForUserEvent( + wheelEvent->mDeltaY < 0 ? value + step : value - step); FireChangeEventIfNeeded(); aVisitor.mEvent->PreventDefault(); } - } else if (mType == FormControlType::InputRange && - nsContentUtils::IsFocusedContent(this) && - GetMinimum() < GetMaximum()) { - Decimal value = GetValueAsDecimal(); - Decimal step = GetStep(); - if (step == kStepAny) { - step = GetDefaultStep(); - } - MOZ_ASSERT(value.isFinite() && step.isFinite()); - SetValueOfRangeForUserEvent( - wheelEvent->mDeltaY < 0 ? value + step : value - step); - FireChangeEventIfNeeded(); - aVisitor.mEvent->PreventDefault(); } } break; } -#endif case eMouseClick: { if (!aVisitor.mEvent->DefaultPrevented() && aVisitor.mEvent->IsTrusted() && diff --git a/dom/html/test/mochitest.toml b/dom/html/test/mochitest.toml index ebafcacf1125..6405da7bbe59 100644 --- a/dom/html/test/mochitest.toml +++ b/dom/html/test/mochitest.toml @@ -773,24 +773,10 @@ skip-if = [ ] ["test_bug1261673.html"] -skip-if = [ - "os == 'android'", - "apple_catalina", - "apple_silicon", -] ["test_bug1261674-1.html"] -skip-if = [ - "os == 'android'", - "apple_catalina", - "apple_silicon", -] ["test_bug1261674-2.html"] -skip-if = [ - "apple_catalina", - "apple_silicon", -] ["test_bug1264157.html"] diff --git a/dom/html/test/test_bug1261673.html b/dom/html/test/test_bug1261673.html index f1ca0eb15386..ab95932e276a 100644 --- a/dom/html/test/test_bug1261673.html +++ b/dom/html/test/test_bug1261673.html @@ -52,6 +52,10 @@ function runTests() { let actualChangeCount = 0; let expectedChangeCount = 0; + const prefName = "dom.input.number_and_range_modified_by_mousewheel"; + let didFlipPref = false; + let isPrefEnabled = SpecialPowers.getBoolPref(prefName); + is(isPrefEnabled, false, "Expecting pref to be disabled by default"); input.addEventListener("change", () => { ++actualChangeCount; }); @@ -59,22 +63,35 @@ function runTests() { function runNext() { let p = params[testIdx]; (p.focus) ? input.focus() : input.blur(); - if (p.valueChanged != 0) { + if (isPrefEnabled && p.valueChanged != 0) { expectedChangeCount++; expectedValue += p.valueChanged; } sendWheelAndPaint(input, 1, 1, { deltaY: p.deltaY, deltaMode: p.deltaMode }, - () => { + async () => { is(parseInt(input.value), expectedValue, - "Handle wheel in number input test-" + testIdx); + "Handle wheel in number input test-" + testIdx + + " with pref " + (isPrefEnabled ? "en" : "dis") + "abled"); + is(actualChangeCount, expectedChangeCount, "UA should fire change event when input's value changed"); if (++testIdx < params.length) { // More subtests remain; kick off the next one. runNext(); + } else if (!didFlipPref) { + // Reached the end of the subtest list. Flip the pref + // and restart our iteration over the subtests. + await SpecialPowers.pushPrefEnv({ + set: [[prefName, !isPrefEnabled]], + }); + isPrefEnabled = !isPrefEnabled; + didFlipPref = true; + testIdx = actualChangeCount = expectedChangeCount = 0; + runNext(); } else { - // Reached the end of the subtest list. We're done! + // Reached the end of the subtest list, for both pref settings. + // We're done! SimpleTest.finish(); } }); diff --git a/dom/html/test/test_bug1261674-1.html b/dom/html/test/test_bug1261674-1.html index 47ff80d71456..69258fd2bd0f 100644 --- a/dom/html/test/test_bug1261674-1.html +++ b/dom/html/test/test_bug1261674-1.html @@ -52,6 +52,10 @@ function runTests() { let actualChangeCount = 0; let expectedChangeCount = 0; + const prefName = "dom.input.number_and_range_modified_by_mousewheel"; + let didFlipPref = false; + let isPrefEnabled = SpecialPowers.getBoolPref(prefName); + is(isPrefEnabled, false, "Expecting pref to be disabled by default"); input.addEventListener("change", () => { ++actualChangeCount; }); @@ -59,22 +63,35 @@ function runTests() { function runNext() { let p = params[testIdx]; (p.focus) ? input.focus() : input.blur(); - if (p.valueChanged != 0) { + if (isPrefEnabled && p.valueChanged != 0) { expectedChangeCount++; expectedValue += p.valueChanged; } sendWheelAndPaint(input, 1, 1, { deltaY: p.deltaY, deltaMode: p.deltaMode }, - () => { + async () => { is(parseInt(input.value), expectedValue, - "Handle wheel in range input test-" + testIdx); + "Handle wheel in range input test-" + testIdx + + " with pref " + (isPrefEnabled ? "en" : "dis") + "abled"); + is(actualChangeCount, expectedChangeCount, "UA should fire change event when input's value changed"); if (++testIdx < params.length) { // More subtests remain; kick off the next one. runNext(); + } else if (!didFlipPref) { + // Reached the end of the subtest list. Flip the pref + // and restart our iteration over the subtests. + await SpecialPowers.pushPrefEnv({ + set: [[prefName, !isPrefEnabled]], + }); + isPrefEnabled = !isPrefEnabled; + didFlipPref = true; + testIdx = actualChangeCount = expectedChangeCount = 0; + runNext(); } else { - // Reached the end of the subtest list. We're done! + // Reached the end of the subtest list, for both pref settings. + // We're done! SimpleTest.finish(); } }); diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index a428aefa1d93..1a89a1745404 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -3013,6 +3013,14 @@ value: @IS_EARLY_BETA_OR_EARLIER@ mirror: always +# Does mousewheel-scrolling over a focused or +# field cause the value to increase/decrease (rather +# than scrolling the page)? +- name: dom.input.number_and_range_modified_by_mousewheel + type: RelaxedAtomicBool + value: false + mirror: always + # Whether to allow or disallow web apps to cancel `beforeinput` events caused # by MozEditableElement#setUserInput() which is used by autocomplete, autofill # and password manager.