Bug 220608

Summary: REGRESSION(r266695) Range control with custom track width sized incorrectly
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, esprehn+autocc, ews-watchlist, glenn, graouts, gyuyoung.kim, hi, koivisto, macpherson, menard, simon.fraser, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=210089
Attachments:
Description Flags
test reduction
none
Patch none

Description zalan 2021-01-13 16:14:26 PST
<rdar://problem/73120825>
Comment 1 zalan 2021-01-13 16:14:40 PST
Created attachment 417577 [details]
test reduction
Comment 2 Simon Fraser (smfr) 2021-01-13 16:44:41 PST
Do we need to change the styling on range inputs to handle this case?
Comment 3 Sergio Villar Senin 2021-01-14 01:40:55 PST
(In reply to Simon Fraser (smfr) from comment #2)
> Do we need to change the styling on range inputs to handle this case?

I think so, the issues that appear with "min-size:auto" changes are normally fixed by setting 0 as the min-size (which was the default before and thus a lot of code relies on that).

Zalan, will this patch provide the expected results:

diff --git a/Source/WebCore/css/html.css b/Source/WebCore/css/html.css
index c2ffebcf0716..011939f1e9ca 100644
--- a/Source/WebCore/css/html.css
+++ b/Source/WebCore/css/html.css
@@ -834,6 +834,7 @@ input[type="range"]::-webkit-slider-container, input[type="range"]::-webkit-medi
     box-sizing: border-box;
     display: flex;
     align-contents: center;
+    min-width: 0;
 }
 
 input[type="range"]::-webkit-slider-runnable-track {
Comment 4 zalan 2021-01-14 10:09:14 PST
It does. It's pretty embarrassing that we've got to add such hacks to make things work while the proper solution is as simple as this

diff --git a/Source/WebCore/rendering/style/RenderStyle.h b/Source/WebCore/rendering/style/RenderStyle.h
index 804454d3e241..50ed6877bf75 100644
--- a/Source/WebCore/rendering/style/RenderStyle.h
+++ b/Source/WebCore/rendering/style/RenderStyle.h
@@ -1551,7 +1551,7 @@ public:
     static float initialLetterSpacing() { return 0; }
     static Length initialWordSpacing() { return Length(Fixed); }
     static Length initialSize() { return Length(); }
-    static Length initialMinSize() { return Length(); }
+    static Length initialMinSize() { return Length(Fixed); }
     static Length initialMaxSize() { return Length(Undefined); }
     static Length initialOffset() { return Length(); }
     static Length initialRadius() { return Length(); }
Comment 5 Sergio Villar Senin 2021-01-15 07:12:47 PST
(In reply to zalan from comment #4)
> It does. It's pretty embarrassing that we've got to add such hacks to make
> things work while the proper solution is as simple as this
> 
> diff --git a/Source/WebCore/rendering/style/RenderStyle.h
> b/Source/WebCore/rendering/style/RenderStyle.h
> index 804454d3e241..50ed6877bf75 100644
> --- a/Source/WebCore/rendering/style/RenderStyle.h
> +++ b/Source/WebCore/rendering/style/RenderStyle.h
> @@ -1551,7 +1551,7 @@ public:
>      static float initialLetterSpacing() { return 0; }
>      static Length initialWordSpacing() { return Length(Fixed); }
>      static Length initialSize() { return Length(); }
> -    static Length initialMinSize() { return Length(); }
> +    static Length initialMinSize() { return Length(Fixed); }
>      static Length initialMaxSize() { return Length(Undefined); }
>      static Length initialOffset() { return Length(); }
>      static Length initialRadius() { return Length(); }

Well but we cannot do this right? Unless I'm missing something the initial value for min-size properties must be auto.
Comment 6 zalan 2021-01-15 08:12:42 PST
(In reply to Sergio Villar Senin from comment #5)
> (In reply to zalan from comment #4)
> > It does. It's pretty embarrassing that we've got to add such hacks to make
> > things work while the proper solution is as simple as this
> > 
> > diff --git a/Source/WebCore/rendering/style/RenderStyle.h
> > b/Source/WebCore/rendering/style/RenderStyle.h
> > index 804454d3e241..50ed6877bf75 100644
> > --- a/Source/WebCore/rendering/style/RenderStyle.h
> > +++ b/Source/WebCore/rendering/style/RenderStyle.h
> > @@ -1551,7 +1551,7 @@ public:
> >      static float initialLetterSpacing() { return 0; }
> >      static Length initialWordSpacing() { return Length(Fixed); }
> >      static Length initialSize() { return Length(); }
> > -    static Length initialMinSize() { return Length(); }
> > +    static Length initialMinSize() { return Length(Fixed); }
> >      static Length initialMaxSize() { return Length(Undefined); }
> >      static Length initialOffset() { return Length(); }
> >      static Length initialRadius() { return Length(); }
> 
> Well but we cannot do this right? Unless I'm missing something the initial
> value for min-size properties must be auto.

Yeah, I didn't upload this as a patch, did I (j/k) :)
Comment 7 Sergio Villar Senin 2021-01-18 00:40:13 PST
Created attachment 417808 [details]
Patch
Comment 8 zalan 2021-01-19 14:09:31 PST
Comment on attachment 417808 [details]
Patch

It looks ready to be landed. Let me just cq+-it. Hope you don't mind it.
Comment 9 EWS 2021-01-19 14:15:16 PST
Committed r271618: <https://trac.webkit.org/changeset/271618>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417808 [details].