...
<rdar://problem/72399087>
Created attachment 423220 [details] Patch
Committed r274473: <https://commits.webkit.org/r274473> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423220 [details].
Comment on attachment 423220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423220&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:350 > + style.setOpacity(0.75f); You can't just set opacity in the style without calling Adjuster::adjust() - opacity has side effects, like creating CSS stacking context.
Re-opened since this is blocked by bug 223287
Created attachment 423746 [details] Patch
Comment on attachment 423746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423746&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:2124 > + if (pressed) > + context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemBlue, styleColorOptions).colorWithAlphaMultipliedBy(pressedStateOpacity)); > + else if (enabled) > context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemBlue, styleColorOptions)); > else > context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemSecondaryFillDisabled, styleColorOptions)); With code like this, it's much better to factor it so that you only call the actual function once. Prepare the inputs based on state, but just have one call to context.fillRoundedRect()
Created attachment 423759 [details] Patch
(In reply to Simon Fraser (smfr) from comment #7) > Comment on attachment 423746 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423746&action=review > > > Source/WebCore/rendering/RenderThemeIOS.mm:2124 > > + if (pressed) > > + context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemBlue, styleColorOptions).colorWithAlphaMultipliedBy(pressedStateOpacity)); > > + else if (enabled) > > context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemBlue, styleColorOptions)); > > else > > context.fillRoundedRect(checkboxRect, systemColor(CSSValueAppleSystemSecondaryFillDisabled, styleColorOptions)); > > With code like this, it's much better to factor it so that you only call the > actual function once. Prepare the inputs based on state, but just have one > call to context.fillRoundedRect() Done. I moved the color determination into separate methods, so that they can be used for both checkboxes and radio buttons.
Comment on attachment 423759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423759&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:2100 > + if (!(states & ControlStates::EnabledState)) ControlStates::States should be an enum class, and be used OptionSet<>
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 423759 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423759&action=review > > > Source/WebCore/rendering/RenderThemeIOS.mm:2100 > > + if (!(states & ControlStates::EnabledState)) > > ControlStates::States should be an enum class, and be used OptionSet<> Tracking this refactoring in https://bugs.webkit.org/show_bug.cgi?id=223647.
Committed r274886: <https://commits.webkit.org/r274886> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423759 [details].