Bug 245156 – Align "UA" stylesheet for "hr", "fieldset", "marquee", "details" etc. with HTML Spec

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Ahmad Saleem]

Hi Team,

I was going through "html.css" UA stylesheet and noticed following:

1) marquee element is missing "text-align: initial".

https://html.spec.whatwg.org/#the-marquee-element-2

2) hr element is missing "overflow: hidden".

https://html.spec.whatwg.org/#the-hr-element-2

3) fieldset element needs to change to "min-inline-size: min-content;" from "min-width: min-content;"

https://html.spec.whatwg.org/#the-fieldset-and-legend-elements

4) details and summary elements to be aligned:

https://html.spec.whatwg.org/#the-details-and-summary-elements

Also bring change of Chrome / Blink, which is also in Firefox / Gecko about:

details > summary:first-of-type {
    display: list-item;
    counter-increment: list-item 0;
    list-style: disclosure-closed inside;
}

_____

I will try to do Pull Request and see if it works without any breakage. If it would be too complex, I would leave it for others to do.

Thanks!

[Karl Dubost]

For reference

=================================
WebKit
https://searchfox.org/wubkat/source/Source/WebCore/css/html.css

marquee {
    display: inline-block;
}

hr {
    display: block;
    margin-block-start: 0.5em;
    margin-block-end: 0.5em;
    margin-inline-start: auto;
    margin-inline-end: auto;
    border-style: inset;
    border-width: 1px;
}

fieldset {
    display: block;
    margin-inline-start: 2px;
    margin-inline-end: 2px;
    padding-block-start: 0.35em;
    padding-inline-start: 0.75em;
    padding-inline-end: 0.75em;
    padding-block-end: 0.625em;
    border: 2px groove ThreeDFace;
    min-width: min-content;
}


=================================
Blink
https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/html/resources/html.css

marquee {
    display: inline-block;
    width: -webkit-fill-available;
}

hr {
    display: block;
    overflow: hidden;
    unicode-bidi: isolate;
    margin-block-start: 0.5em;
    margin-block-end: 0.5em;
    margin-inline-start: auto;
    margin-inline-end: auto;
    border-style: inset;
    border-width: 1px
}

fieldset {
    display: block;
    margin-inline-start: 2px;
    margin-inline-end: 2px;
    padding-block-start: 0.35em;
    padding-inline-start: 0.75em;
    padding-inline-end: 0.75em;
    padding-block-end: 0.625em;
    border: 2px groove #C0C0C0;
    min-inline-size: min-content;
}


=================================
Gecko 
https://searchfox.org/mozilla-central/source/layout/style/res/html.css

marquee {
  inline-size: -moz-available;
  display: inline-block;
  vertical-align: text-bottom;
  text-align: start;
}

marquee:is([direction="up"], [direction="down"]) {
  block-size: 200px;
}

/* <hr> noshade and color attributes are handled completely by
 * HTMLHRElement::MapAttributesIntoRule.
 * https://html.spec.whatwg.org/#the-hr-element-2
 */
hr {
  color: gray;
  border-width: 1px;
  border-style: inset;
  margin-block-start: 0.5em;
  margin-block-end: 0.5em;
  margin-inline-start: auto;
  margin-inline-end: auto;
  overflow: hidden;

  /* FIXME: This is not really per spec */
  display: block;
}

hr[size="1"] {
  border-style: solid none none none;
}


For details and summary it will be slightly complex, because there are specific behaviors associated to this. See Bug 157323

I have the feeling that would be probably addressing each of them separately instead of one single PR.

[Ahmad Saleem]

(In reply to Karl Dubost from comment #1)
> For reference
> 
> =================================
> WebKit
> https://searchfox.org/wubkat/source/Source/WebCore/css/html.css
> 
> marquee {
>     display: inline-block;
> }
> 
> hr {
>     display: block;
>     margin-block-start: 0.5em;
>     margin-block-end: 0.5em;
>     margin-inline-start: auto;
>     margin-inline-end: auto;
>     border-style: inset;
>     border-width: 1px;
> }
> 
> fieldset {
>     display: block;
>     margin-inline-start: 2px;
>     margin-inline-end: 2px;
>     padding-block-start: 0.35em;
>     padding-inline-start: 0.75em;
>     padding-inline-end: 0.75em;
>     padding-block-end: 0.625em;
>     border: 2px groove ThreeDFace;
>     min-width: min-content;
> }
> 
> 
> =================================
> Blink
> https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/
> core/html/resources/html.css
> 
> marquee {
>     display: inline-block;
>     width: -webkit-fill-available;
> }
> 
> hr {
>     display: block;
>     overflow: hidden;
>     unicode-bidi: isolate;
>     margin-block-start: 0.5em;
>     margin-block-end: 0.5em;
>     margin-inline-start: auto;
>     margin-inline-end: auto;
>     border-style: inset;
>     border-width: 1px
> }
> 
> fieldset {
>     display: block;
>     margin-inline-start: 2px;
>     margin-inline-end: 2px;
>     padding-block-start: 0.35em;
>     padding-inline-start: 0.75em;
>     padding-inline-end: 0.75em;
>     padding-block-end: 0.625em;
>     border: 2px groove #C0C0C0;
>     min-inline-size: min-content;
> }
> 
> 
> =================================
> Gecko 
> https://searchfox.org/mozilla-central/source/layout/style/res/html.css
> 
> marquee {
>   inline-size: -moz-available;
>   display: inline-block;
>   vertical-align: text-bottom;
>   text-align: start;
> }
> 
> marquee:is([direction="up"], [direction="down"]) {
>   block-size: 200px;
> }
> 
> /* <hr> noshade and color attributes are handled completely by
>  * HTMLHRElement::MapAttributesIntoRule.
>  * https://html.spec.whatwg.org/#the-hr-element-2
>  */
> hr {
>   color: gray;
>   border-width: 1px;
>   border-style: inset;
>   margin-block-start: 0.5em;
>   margin-block-end: 0.5em;
>   margin-inline-start: auto;
>   margin-inline-end: auto;
>   overflow: hidden;
> 
>   /* FIXME: This is not really per spec */
>   display: block;
> }
> 
> hr[size="1"] {
>   border-style: solid none none none;
> }
> 
> 
> For details and summary it will be slightly complex, because there are
> specific behaviors associated to this. See Bug 157323
> 
> I have the feeling that would be probably addressing each of them separately
> instead of one single PR.

Yes - I have already created separate bugs for "fieldset" and "marquee", will ignore "details and summary" for now but will try to do "hr" but I noticed that there were a lot of failures let's see.

[Ahmad Saleem]

I tried to land this in one go but failed so I did split patches:

PR of failed attempt for reference: https://github.com/WebKit/WebKit/pull/4327

I am able to do changes for 'fieldset' and 'marquee' - please refer to 'See Also' section.

___

Learning:

'hr' element change is troublesome because it leads to "render" layer being added and had to re baseline 200+ tests.

Although rebaselining is not the difficult part:

-> on 'win' bot, it leads to change in outline color being changed from 'blue' to 'black' and I don't know how to fix it.
-> on some WPT test cases in 'SELECTION', we start to fail them and they time out.

Although it leads to some passes as well.

___

For Details & Summary - I will try to land them in separate during this week without 'list-item' to see at least, it works for remaining cases.

Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/100185628>

[Karl Dubost]

Ahmad,

> 'hr' element change is troublesome because it leads to "render" layer being added and had to re baseline 200+ tests.

the HR patch has been landed.
https://github.com/WebKit/WebKit/pull/4724

[Ahmad Saleem]

(In reply to Karl Dubost from comment #5)
> Ahmad,
> 
> > 'hr' element change is troublesome because it leads to "render" layer being added and had to re baseline 200+ tests.
> 
> the HR patch has been landed.
> https://github.com/WebKit/WebKit/pull/4724

Yes - I think we can close this bug since now pretty much everything is done here except details, which is dependent on bug 157323.