...
<rdar://problem/79456679>
Created attachment 431695 [details] For EWS
Could the layout tests cover the IC variant?
Relating bug 227145 as tracking the checkboxes, etc.
Comment on attachment 431695 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=431695&action=review > Source/WebCore/css/html.css:417 > + border: 1px solid -apple-system-opaque-separator; From another thread, this should be -apple-system-separator since it has an IC variant.
Created attachment 431864 [details] Patch
(In reply to James Craig from comment #3) > Could the layout tests cover the IC variant? The layout tests update are old render tree dumps, I would prefer not to add new render tree dump tests or to modify existing ones. We could update fast/css/ios/system-color-for-css-value.html, but that would require new logic to force IC mode in tests. I've filed https://bugs.webkit.org/show_bug.cgi?id=227218 to track adding the mechanism / tests.
Comment on attachment 431864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431864&action=review > Source/WebCore/css/html.css:417 > + border: 1px solid -apple-system-opaque-separator; It looks like James's suggestion to use -apple-system-separator here instead has not been applied.
(In reply to Maciej Stachowiak from comment #9) > Comment on attachment 431864 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=431864&action=review > > > Source/WebCore/css/html.css:417 > > + border: 1px solid -apple-system-opaque-separator; > > It looks like James's suggestion to use -apple-system-separator here instead > has not been applied. We still want an opaque color, hence the use of "-apple-system-opaque-separator". "-apple-system-separator" is a transparent color. From the other thread, we actually want to use separatorColor composited on top of a solid color (essentially creating our own opaque separator, since the UIKit version is currently missing an Increased Contrast variant). Consequently, the change that James mentions is actually applied in the RenderThemeIOS change in the patch, where we now use separatorColor with the compositing behavior. The name of the CSS value is kept the same to reflect the opaqueness.
Comment on attachment 431864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431864&action=review r=me but see question. >>>> Source/WebCore/css/html.css:417 >>>> + border: 1px solid -apple-system-opaque-separator; >>> >>> It looks like James's suggestion to use -apple-system-separator here instead has not been applied. >> >> It looks like James's suggestion to use -apple-system-separator here instead has not been applied. > > We still want an opaque color, hence the use of "-apple-system-opaque-separator". "-apple-system-separator" is a transparent color. > > From the other thread, we actually want to use separatorColor composited on top of a solid color (essentially creating our own opaque separator, since the UIKit version is currently missing an Increased Contrast variant). > > Consequently, the change that James mentions is actually applied in the RenderThemeIOS change in the patch, where we now use separatorColor with the compositing behavior. The name of the CSS value is kept the same to reflect the opaqueness. I see! I'm not totally sure how the change in RenderThemeIOS.mm does that. Does the ", true" force compositing on top of a solid color? Does that handle dark mode in a reasonable way (as pointed out in another comment in this Radar)?
(In reply to Maciej Stachowiak from comment #11) > Comment on attachment 431864 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=431864&action=review > > r=me but see question. > > >>>> Source/WebCore/css/html.css:417 > >>>> + border: 1px solid -apple-system-opaque-separator; > >>> > >>> It looks like James's suggestion to use -apple-system-separator here instead has not been applied. > >> > >> It looks like James's suggestion to use -apple-system-separator here instead has not been applied. > > > > We still want an opaque color, hence the use of "-apple-system-opaque-separator". "-apple-system-separator" is a transparent color. > > > > From the other thread, we actually want to use separatorColor composited on top of a solid color (essentially creating our own opaque separator, since the UIKit version is currently missing an Increased Contrast variant). > > > > Consequently, the change that James mentions is actually applied in the RenderThemeIOS change in the patch, where we now use separatorColor with the compositing behavior. The name of the CSS value is kept the same to reflect the opaqueness. > > I see! I'm not totally sure how the change in RenderThemeIOS.mm does that. > Does the ", true" force compositing on top of a solid color? Does that > handle dark mode in a reasonable way (as pointed out in another comment in > this Radar)? Yes, the compositing behavior activated with the ", true". The way we handle dark mode is by compositing the dark mode variant of the color over solid black (logic in `systemColorFromCSSValueSystemColorInformation`). Thanks for the review!
Committed r279097 (239014@main): <https://commits.webkit.org/239014@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431864 [details].