Bug 238751 - [css-ui] Hide input[type=search] icons when appearance: textfield is set
Summary: [css-ui] Hide input[type=search] icons when appearance: textfield is set
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Nguyen (:ntim)
URL:
Keywords: InRadar
Depends on: 238551 240798
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-04 11:53 PDT by Tim Nguyen (:ntim)
Modified: 2022-08-23 18:39 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Nguyen (:ntim) 2022-04-04 11:53:43 PDT
https://drafts.csswg.org/css-ui-4/#valdef-appearance-textfield

textfield
For input elements where the type attribute is in the Search state, the element is rendered as a "normal" text entry widget, similar to an input element where the type attribute is in the Text state.
For all other elements, this value has the same effect as auto.
Comment 1 Radar WebKit Bug Importer 2022-04-04 11:53:56 PDT
<rdar://problem/91253463>
Comment 2 Tim Nguyen (:ntim) 2022-04-04 13:47:40 PDT
This is a good starting point to hide the clear button:

diff --git a/Source/WebCore/html/shadow/TextControlInnerElements.cpp b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
index c0e8fee503fe..0098997ddc63 100644
--- a/Source/WebCore/html/shadow/TextControlInnerElements.cpp
+++ b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
@@ -306,11 +306,13 @@ Ref<SearchFieldCancelButtonElement> SearchFieldCancelButtonElement::create(Docum
     return element;
 }
 
-std::optional<Style::ElementStyle> SearchFieldCancelButtonElement::resolveCustomStyle(const Style::ResolutionContext& resolutionContext, const RenderStyle*)
+std::optional<Style::ElementStyle> SearchFieldCancelButtonElement::resolveCustomStyle(const Style::ResolutionContext& resolutionContext, const RenderStyle* shadowHostStyle)
 {
     auto elementStyle = resolveStyle(resolutionContext);
     auto& inputElement = downcast<HTMLInputElement>(*shadowHost());
     elementStyle.renderStyle->setVisibility(elementStyle.renderStyle->visibility() == Visibility::Hidden || inputElement.value().isEmpty() ? Visibility::Hidden : Visibility::Visible);
+    if (shadowHostStyle->effectiveAppearance() == TextFieldPart)
+        elementStyle.renderStyle->setEffectiveDisplay(DisplayType::None);
     return elementStyle;
 }



Remaining issues are:
- width is too large compared to <input type="text"> (not quite sure why)
- focusring offset is too large compared to <input type="text"> (html.css sets input[type=text] outline-offset to -2px, but [type=search] to 0, outline-offset needs to be tied to appearance: textfield instead of input[type])
Comment 3 zsun 2022-04-06 07:41:34 PDT
(In reply to Tim Nguyen (:ntim) from comment #2)
> This is a good starting point to hide the clear button:
> 
> diff --git a/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> index c0e8fee503fe..0098997ddc63 100644
> --- a/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> +++ b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> @@ -306,11 +306,13 @@ Ref<SearchFieldCancelButtonElement>
> SearchFieldCancelButtonElement::create(Docum
>      return element;
>  }
>  
> -std::optional<Style::ElementStyle>
> SearchFieldCancelButtonElement::resolveCustomStyle(const
> Style::ResolutionContext& resolutionContext, const RenderStyle*)
> +std::optional<Style::ElementStyle>
> SearchFieldCancelButtonElement::resolveCustomStyle(const
> Style::ResolutionContext& resolutionContext, const RenderStyle*
> shadowHostStyle)
>  {
>      auto elementStyle = resolveStyle(resolutionContext);
>      auto& inputElement = downcast<HTMLInputElement>(*shadowHost());
>     
> elementStyle.renderStyle->setVisibility(elementStyle.renderStyle-
> >visibility() == Visibility::Hidden || inputElement.value().isEmpty() ?
> Visibility::Hidden : Visibility::Visible);
> +    if (shadowHostStyle->effectiveAppearance() == TextFieldPart)
> +        elementStyle.renderStyle->setEffectiveDisplay(DisplayType::None);
>      return elementStyle;
>  }
> 
> 
> 
> Remaining issues are:
> - width is too large compared to <input type="text"> (not quite sure why)
> - focusring offset is too large compared to <input type="text"> (html.css
> sets input[type=text] outline-offset to -2px, but [type=search] to 0,
> outline-offset needs to be tied to appearance: textfield instead of
> input[type])

I figure the remaining issues might still be related to the SearchFieldCancelButtonElement.  The extra width seems caused by the SearchFieldCancelButtonElement. When we do "elementStyle.renderStyle->setEffectiveDisplay(DisplayType::None);", is it supposed to re-rendering the box or just simply make the CancelButton invisible (but it still exists)? I'm not familiar with this code...
Comment 4 zalan 2022-04-06 08:39:52 PDT
(In reply to zsun from comment #3)
> (In reply to Tim Nguyen (:ntim) from comment #2)
> > This is a good starting point to hide the clear button:
> > 
> > diff --git a/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> > b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> > index c0e8fee503fe..0098997ddc63 100644
> > --- a/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> > +++ b/Source/WebCore/html/shadow/TextControlInnerElements.cpp
> > @@ -306,11 +306,13 @@ Ref<SearchFieldCancelButtonElement>
> > SearchFieldCancelButtonElement::create(Docum
> >      return element;
> >  }
> >  
> > -std::optional<Style::ElementStyle>
> > SearchFieldCancelButtonElement::resolveCustomStyle(const
> > Style::ResolutionContext& resolutionContext, const RenderStyle*)
> > +std::optional<Style::ElementStyle>
> > SearchFieldCancelButtonElement::resolveCustomStyle(const
> > Style::ResolutionContext& resolutionContext, const RenderStyle*
> > shadowHostStyle)
> >  {
> >      auto elementStyle = resolveStyle(resolutionContext);
> >      auto& inputElement = downcast<HTMLInputElement>(*shadowHost());
> >     
> > elementStyle.renderStyle->setVisibility(elementStyle.renderStyle-
> > >visibility() == Visibility::Hidden || inputElement.value().isEmpty() ?
> > Visibility::Hidden : Visibility::Visible);
> > +    if (shadowHostStyle->effectiveAppearance() == TextFieldPart)
> > +        elementStyle.renderStyle->setEffectiveDisplay(DisplayType::None);
> >      return elementStyle;
> >  }
> > 
> > 
> > 
> > Remaining issues are:
> > - width is too large compared to <input type="text"> (not quite sure why)
> > - focusring offset is too large compared to <input type="text"> (html.css
> > sets input[type=text] outline-offset to -2px, but [type=search] to 0,
> > outline-offset needs to be tied to appearance: textfield instead of
> > input[type])
> 
> I figure the remaining issues might still be related to the
> SearchFieldCancelButtonElement.  The extra width seems caused by the
> SearchFieldCancelButtonElement. When we do
> "elementStyle.renderStyle->setEffectiveDisplay(DisplayType::None);", is it
> supposed to re-rendering the box or just simply make the CancelButton
> invisible (but it still exists)? I'm not familiar with this code...
Not sure what you mean by re-rendering and/or invisible but display:none means no associated renderer which implies no geometry (i.e. does not take up any space. it is indeed invisible but not in the sense of CSS visibility but more like "it does not participate in layout at all". When an element gains "display: none", the associated renderer needs to be detached/destroyed and layout to be issued).
Comment 5 Tim Nguyen (:ntim) 2022-05-22 11:34:11 PDT
Pull request: https://github.com/WebKit/WebKit/pull/895
Comment 6 EWS 2022-08-23 12:08:40 PDT
Committed 253691@main (d08a6f782660): <https://commits.webkit.org/253691@main>

Reviewed commits have been landed. Closing PR #895 and removing active labels.