Bug 227135 – [iOS] Update textfield border color to match specification

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Aditya Keerthi]

...

[Aditya Keerthi]

<rdar://problem/79456679>

[Aditya Keerthi]

Created attachment 431695 [details]
For EWS

[James Craig]

Could the layout tests cover the IC variant?

[James Craig]

Relating bug 227145 as tracking the checkboxes, etc.

[James Craig]

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.

[Aditya Keerthi]

Created attachment 431864 [details]
Patch

[Aditya Keerthi]

(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.

[Maciej Stachowiak]

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.

[Maciej Stachowiak]

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.

[Aditya Keerthi]

(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.

[Maciej Stachowiak]

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)?

[Aditya Keerthi]

(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!

[EWS]

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].