Skip to content

Commit

Permalink
Limit non-initial <input type=checkbox switch> styling to appearance:…
Browse files Browse the repository at this point in the history
…auto

https://bugs.webkit.org/show_bug.cgi?id=267334

Reviewed by Aditya Keerthi and Tim Nguyen.

Per discussion in whatwg/html#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
  • Loading branch information
annevk committed Jan 16, 2024
1 parent 57adc66 commit ac733e6
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 7 deletions.
6 changes: 6 additions & 0 deletions LayoutTests/fast/forms/switch/display-expected.txt
Original file line number Diff line number Diff line change
@@ -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"

20 changes: 20 additions & 0 deletions LayoutTests/fast/forms/switch/display.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<script src="../../../resources/js-test-pre.js"></script>
<input type=checkbox switch>
<script>
const control = document.querySelector("input");

control.style.display = "none";
shouldBeEqualToString("getComputedStyle(control).display", "none");

control.style.display = "inline";
shouldBeEqualToString("getComputedStyle(control).display", "inline-grid");

control.style.display = "table";
shouldBeEqualToString("getComputedStyle(control).display", "grid");

control.style.display = "block";
shouldBeEqualToString("getComputedStyle(control).display", "grid");

control.style.display = "inline-grid";
shouldBeEqualToString("getComputedStyle(control).display", "inline-grid");
</script>
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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");
16 changes: 16 additions & 0 deletions LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/css/htmlSwitchControl.css
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions Source/WebCore/rendering/RenderTheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/rendering/RenderTheme.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
12 changes: 7 additions & 5 deletions Source/WebCore/rendering/RenderThemeIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -723,12 +723,14 @@ static bool renderThemePaintSwitchTrack(OptionSet<ControlStyle::State>, 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);
Expand Down

0 comments on commit ac733e6

Please sign in to comment.