From ac733e6cb304cd07c2456b71f170d8758755aad6 Mon Sep 17 00:00:00 2001 From: Anne van Kesteren Date: Tue, 16 Jan 2024 10:44:22 -0800 Subject: [PATCH] Limit non-initial styling to appearance:auto https://bugs.webkit.org/show_bug.cgi?id=267334 Reviewed by Aditya Keerthi and Tim Nguyen. Per discussion in https://github.com/whatwg/html/issues/4180 we do not want styles intended for appearance:auto to leak into appearance:none. Ideally we pay more attention to this with all form controls going forward, but that is a much larger undertaking. * LayoutTests/fast/forms/switch/display-expected.txt: Added. * LayoutTests/fast/forms/switch/display.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window.js: * LayoutTests/platform/ios/TestExpectations: Enable WebKit-specific switch tests on iOS too, except for those that currently do not work. * Source/WebCore/css/htmlSwitchControl.css: (@namespace "http://www.w3.org/1999/xhtml";): (input[type="checkbox"][switch]::thumb, input[type="checkbox"][switch]::track): * Source/WebCore/rendering/RenderTheme.cpp: (WebCore::RenderTheme::adjustStyle): (WebCore::RenderTheme::adjustSwitchStyleDisplay const): (WebCore::RenderTheme::adjustSwitchStyle const): (WebCore::RenderTheme::adjustSwitchThumbOrSwitchTrackStyle const): * Source/WebCore/rendering/RenderTheme.h: * Source/WebCore/rendering/RenderThemeIOS.mm: (WebCore::RenderThemeIOS::adjustSwitchStyle const): No longer skip most of the iOS style adjusting logic for non-auto sizes. Canonical link: https://commits.webkit.org/273078@main --- .../fast/forms/switch/display-expected.txt | 6 ++++ LayoutTests/fast/forms/switch/display.html | 20 +++++++++++++ ...ckbox-switch.tentative.window-expected.txt | 2 ++ .../input-checkbox-switch.tentative.window.js | 18 ++++++++++++ LayoutTests/platform/ios/TestExpectations | 16 ++++++++++ Source/WebCore/css/htmlSwitchControl.css | 3 +- Source/WebCore/rendering/RenderTheme.cpp | 29 +++++++++++++++++++ Source/WebCore/rendering/RenderTheme.h | 2 ++ Source/WebCore/rendering/RenderThemeIOS.mm | 12 ++++---- 9 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 LayoutTests/fast/forms/switch/display-expected.txt create mode 100644 LayoutTests/fast/forms/switch/display.html diff --git a/LayoutTests/fast/forms/switch/display-expected.txt b/LayoutTests/fast/forms/switch/display-expected.txt new file mode 100644 index 0000000000000..e8f6d3f624b4b --- /dev/null +++ b/LayoutTests/fast/forms/switch/display-expected.txt @@ -0,0 +1,6 @@ +PASS getComputedStyle(control).display is "none" +PASS getComputedStyle(control).display is "inline-grid" +PASS getComputedStyle(control).display is "grid" +PASS getComputedStyle(control).display is "grid" +PASS getComputedStyle(control).display is "inline-grid" + diff --git a/LayoutTests/fast/forms/switch/display.html b/LayoutTests/fast/forms/switch/display.html new file mode 100644 index 0000000000000..300970f774958 --- /dev/null +++ b/LayoutTests/fast/forms/switch/display.html @@ -0,0 +1,20 @@ + + + diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window-expected.txt index 04cc22e13baec..9962530e8bdc6 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window-expected.txt @@ -1,4 +1,6 @@ PASS Default appearance value +PASS Default appearance value: display:none PASS appearance:none should work +PASS appearance:none should work: display gets its initial value diff --git a/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window.js b/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window.js index 7e5095a337815..84198eda100ad 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window.js +++ b/LayoutTests/imported/w3c/web-platform-tests/html/rendering/widgets/input-checkbox-switch.tentative.window.js @@ -6,6 +6,15 @@ test(t => { assert_equals(getComputedStyle(input).appearance, "auto"); }, "Default appearance value"); +test(t => { + const input = document.body.appendChild(document.createElement("input")); + t.add_cleanup(() => input.remove()); + input.type = "checkbox"; + input.switch = true; + input.style.display = "none" + assert_equals(getComputedStyle(input).display, "none"); +}, "Default appearance value: display:none"); + test(t => { const input = document.body.appendChild(document.createElement("input")); t.add_cleanup(() => input.remove()); @@ -14,3 +23,12 @@ test(t => { input.style.appearance = "none"; assert_equals(getComputedStyle(input).appearance, "none"); }, "appearance:none should work"); + +test(t => { + const input = document.body.appendChild(document.createElement("input")); + t.add_cleanup(() => input.remove()); + input.type = "checkbox"; + input.switch = true; + input.style.appearance = "none"; + assert_equals(getComputedStyle(input).display, "inline"); +}, "appearance:none should work: display gets its initial value"); diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations index fc6b1cb826ac4..933d5a9a44253 100644 --- a/LayoutTests/platform/ios/TestExpectations +++ b/LayoutTests/platform/ios/TestExpectations @@ -27,6 +27,22 @@ fast/events/ios [ Pass ] fast/events/ios/ipad [ Skip ] fast/forms/ios [ Pass ] fast/forms/ios/ipad [ Skip ] +fast/forms/switch [ Pass ] + +# More work required for these tests to run on iOS +fast/forms/switch/click-animation-disabled.html [ Skip ] +fast/forms/switch/click-animation-interrupted-document-removed.html [ Skip ] +fast/forms/switch/click-animation-interrupted-type-change.html [ Skip ] +fast/forms/switch/click-animation-interrupted.html [ Skip ] +fast/forms/switch/click-animation-preventdefault.html [ Skip ] +fast/forms/switch/click-animation-redundant-checked.html [ Skip ] +fast/forms/switch/click-animation-redundant-disabled.html [ Skip ] +fast/forms/switch/click-animation-twice-fast.html [ Skip ] +fast/forms/switch/click-animation-twice.html [ Skip ] +fast/forms/switch/click-animation.html [ Skip ] +fast/forms/switch/pointer-tracking-there-and-back-again-rtl.html [ Skip ] +fast/forms/switch/pointer-tracking-there-and-back-again.html [ Skip ] +fast/forms/switch/pointer-tracking.html [ Skip ] # These tests fail or are flaky in non-internal iOS 14 simulator fast/forms/ios/inputmode-none-with-hardware-keyboard.html [ Pass Failure ] diff --git a/Source/WebCore/css/htmlSwitchControl.css b/Source/WebCore/css/htmlSwitchControl.css index dcfdbba810295..5f724a6bf0b3a 100644 --- a/Source/WebCore/css/htmlSwitchControl.css +++ b/Source/WebCore/css/htmlSwitchControl.css @@ -26,12 +26,11 @@ input[type="checkbox"][switch] { margin: initial; - display: inline-grid; + display: initial; } input[type="checkbox"][switch]::thumb, input[type="checkbox"][switch]::track { appearance: inherit !important; - grid-area: 1/1; } #if defined(WTF_PLATFORM_IOS_FAMILY) && WTF_PLATFORM_IOS_FAMILY diff --git a/Source/WebCore/rendering/RenderTheme.cpp b/Source/WebCore/rendering/RenderTheme.cpp index 433ddc1b74f1a..b3266c24dcc27 100644 --- a/Source/WebCore/rendering/RenderTheme.cpp +++ b/Source/WebCore/rendering/RenderTheme.cpp @@ -261,6 +261,9 @@ void RenderTheme::adjustStyle(RenderStyle& style, const Element* element, const return adjustSearchFieldResultsButtonStyle(style, element); case StyleAppearance::Switch: return adjustSwitchStyle(style, element); + case StyleAppearance::SwitchThumb: + case StyleAppearance::SwitchTrack: + return adjustSwitchThumbOrSwitchTrackStyle(style); case StyleAppearance::ProgressBar: return adjustProgressBarStyle(style, element); case StyleAppearance::Meter: @@ -1501,6 +1504,21 @@ void RenderTheme::adjustSliderThumbStyle(RenderStyle& style, const Element* elem adjustSliderThumbSize(style, element); } +void RenderTheme::adjustSwitchStyleDisplay(RenderStyle& style) const +{ + // RenderTheme::adjustStyle() normalizes a bunch of display types to InlineBlock and Block. + switch (style.display()) { + case DisplayType::InlineBlock: + style.setEffectiveDisplay(DisplayType::InlineGrid); + break; + case DisplayType::Block: + style.setEffectiveDisplay(DisplayType::Grid); + break; + default: + break; + } +} + void RenderTheme::adjustSwitchStyle(RenderStyle& style, const Element*) const { // FIXME: This probably has the same flaw as @@ -1509,6 +1527,17 @@ void RenderTheme::adjustSwitchStyle(RenderStyle& style, const Element*) const auto controlSize = Theme::singleton().controlSize(StyleAppearance::Switch, style.fontCascade(), { style.logicalWidth(), style.logicalHeight() }, style.effectiveZoom()); style.setLogicalWidth(WTFMove(controlSize.width)); style.setLogicalHeight(WTFMove(controlSize.height)); + + adjustSwitchStyleDisplay(style); +} + +void RenderTheme::adjustSwitchThumbOrSwitchTrackStyle(RenderStyle& style) const +{ + GridPosition position; + position.setExplicitPosition(1, nullString()); + + style.setGridItemRowStart(position); + style.setGridItemColumnStart(position); } void RenderTheme::purgeCaches() diff --git a/Source/WebCore/rendering/RenderTheme.h b/Source/WebCore/rendering/RenderTheme.h index 4e9d595b3ccf9..36d66596b8089 100644 --- a/Source/WebCore/rendering/RenderTheme.h +++ b/Source/WebCore/rendering/RenderTheme.h @@ -382,7 +382,9 @@ class RenderTheme { virtual void adjustSearchFieldResultsButtonStyle(RenderStyle&, const Element*) const { } virtual bool paintSearchFieldResultsButton(const RenderBox&, const PaintInfo&, const IntRect&) { return true; } + void adjustSwitchStyleDisplay(RenderStyle&) const; virtual void adjustSwitchStyle(RenderStyle&, const Element*) const; + void adjustSwitchThumbOrSwitchTrackStyle(RenderStyle&) const; virtual bool paintSwitchThumb(const RenderObject&, const PaintInfo&, const FloatRect&) { return true; } virtual bool paintSwitchTrack(const RenderObject&, const PaintInfo&, const FloatRect&) { return true; } diff --git a/Source/WebCore/rendering/RenderThemeIOS.mm b/Source/WebCore/rendering/RenderThemeIOS.mm index a9f57bf47fd84..37c95b3782ba9 100644 --- a/Source/WebCore/rendering/RenderThemeIOS.mm +++ b/Source/WebCore/rendering/RenderThemeIOS.mm @@ -723,12 +723,14 @@ static bool renderThemePaintSwitchTrack(OptionSet, const Re void RenderThemeIOS::adjustSwitchStyle(RenderStyle& style, const Element* element) const { - if (!style.width().isAuto() && !style.height().isAuto()) - return; + // FIXME: Deduplicate sizing with the generic code somehow. + if (style.width().isAuto() || style.height().isAuto()) { + auto size = std::max(style.computedFontSize(), logicalSwitchHeight); + style.setLogicalWidth({ size * (logicalSwitchWidth / logicalSwitchHeight), LengthType::Fixed }); + style.setLogicalHeight({ size, LengthType::Fixed }); + } - auto size = std::max(style.computedFontSize(), logicalSwitchHeight); - style.setLogicalWidth({ size * (logicalSwitchWidth / logicalSwitchHeight), LengthType::Fixed }); - style.setLogicalHeight({ size, LengthType::Fixed }); + adjustSwitchStyleDisplay(style); if (style.outlineStyleIsAuto() == OutlineIsAuto::On) style.setOutlineStyle(BorderStyle::None);