Open Bug 1488228 Opened 6 years ago Updated 2 years ago

<legend align> should map to justify-self

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox63 --- affected

People

(Reporter: zcorpan, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

Testcase #1
513 bytes, text/html
Details
Testcase #2
1.55 KB, text/html
Details
Testcase #3 - <legend> w. auto margins
952 bytes, text/html
Details
wip - map align to justify-self
10.91 KB, patch
Details | Diff | Splinter Review 
See https://github.com/web-platform-tests/wpt/pull/12811

In Firefox text-align is always "start".
Blocks: fieldset
Attached file Testcase #1 — 

    
    

    
  
I'm guessing WebKit/Blink does this mapping, but not Gecko/Edge, correct?
Is there a discussion somewhere that motivates why we should standardize the WebKit/Blink behavior?
Flags: needinfo?(zcorpan)

    
    

    
  
<!DOCTYPE html>
<div align="center">
  <div style="width:50%; background:pink;">
    Block centered within containing block.<br>
    Text centered within block.
  </div>
</div>

Also Gecko sets text-align:center here. Shouldn't we do the same for LEGEND?

    
    

    
  
That's something very specific to div and doesn't happen for any other HTML element afaict:

  https://searchfox.org/mozilla-central/rev/721842eed881c7fcdccb9ec0fe79e4e6d4e46604/dom/html/nsGenericHTMLElement.cpp#1298

    
    

    
  
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
> That's something very specific to div and doesn't happen for any other HTML
> element afaict

Err, I lie: P / headers / table parts and textarea also do it. But I don't see that as an argument to do the same for legend, necessarily.

    
    

    
  

      
      
      
      

        Attached file
          
        Testcase #2
        — 
      

    


  
(In reply to Morten Stenshorne from comment #3)
> Also Gecko sets text-align:center here.

That's not exactly true, we map it to '-moz-center' which has different
behavior from 'center'.  It doesn't inherit into a table child.

Chrome appears to do something similar; it maps it to '-webkit-center'
which also has that special behavior.

So, if anything, we should map it to '-moz-center' (and possibly
rename that to '-webkit-center').

    
    

    
  
Which elements does WebKit/Blink implement this mapping for?

    
    

    
  
Blink applies it to every html element:

  https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/html_element.cc?l=249&rcl=77522d333fa583cf0d91595078f38a68f291dd3f

Example:

  data:text/html,<style>foo { display: block; }</style><foo align="center">am i centered?</foo>

Note that there's an exception for input types other than image:

  https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/html/forms/html_input_element.cc?l=707&rcl=e6df63454fdf61d917b3b345e70b2b7504ca8a7e

    
    

    
  
Not that they map align to 'center' (no '-webkit-center') except for p, table parts and div. What spec backs that up?

    
    

    
  
I don't think mapping align on all elements to text-align is supported by any spec.

The HTML Rendering specifies various mappings on a few HTML elements, in specific situations:
14.3.3 Flow content says:
"The center element, and the div element when it has an align attribute whose value is an ASCII case-insensitive match for either the string "center" or the string "middle", are expected to center text within themselves, as if they had their 'text-align' property set to 'center' in a presentational hint, and to align descendants to the center."

[ditto for left/right/justify]
https://html.spec.whatwg.org/multipage/rendering.html#flow-content-3

14.3.9 Tables says:
"The thead, tbody, tfoot, tr, td, and th elements, when they have an align attribute whose value is an ASCII case-insensitive match for either the string "center" or the string "middle", are expected to center text within themselves, as if they had their 'text-align' property set to 'center' in a presentational hint, and to align descendants to the center."
etc
https://html.spec.whatwg.org/multipage/rendering.html#tables-2

There are also UA and "presentational hint" rules, such as:
"caption { text-align: center; }"
"p[align=center i], h1[align=center i], h2[align=center i], h3[align=center i],
h4[align=center i], h5[align=center i], h6[align=center i] {
  text-align: center;
}"
etc

14.3.13 The fieldset and legend elements
Note that align here has no mapping of align to CSS, it merely says
that the alignment position of the legend is taken directly from its value.
https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements

Various other elements map align=left/right to float:left/right ...
https://html.spec.whatwg.org/multipage/rendering.html#attributes-for-embedded-content-and-images
... or margin:auto
https://html.spec.whatwg.org/multipage/rendering.html#the-hr-element-2
but without affecting text-align AFAICT.

So it seems to me that Gecko is currently far more HTML spec-compliant
than WebKit/Blink is.

We could of course add such a mapping to <legend> but it seems to me
that CSS text-align is poor choice, for two reasons:
1. the align attribute is supposed to align the element *itself* within
   its parent; currently the HTML spec uses float:left/right or
   auto inline margins for that use case, not text-align.
   CSS text-align OTOH aligns the element's *contents*.
   Adding *a new semantic meaning* to text-align seems bad to me.
2. mapping to text-align makes it impossible to align the <legend>
   contents differently than the element itself (see Testcase #1),
   which is a breaking change for Gecko

Perhaps we could try to map it to auto margins instead (similar to <hr>)?
It appears legend auto-margins already works somewhat correctly in
Firefox/Safari/Chrome/Edge.

    
    

    
  
(I filed bug 1488301 to fix our <legend> auto inline margins in any case.)

    
    

    
  
In case we want to go with the WebKit/Blink default text
alignment we could add rules like this in the UA sheet:

legend[align=right i] {
  margin-inline-start: auto;
  margin-inline-end: 0;
  text-align: end;
}

then authors can easily override the text-align value
as they see fit.

    
    

    
  
If we can get away with not specifying more attribute mappings, I'd prefer that. They're kind of slow, and they're supposed to be legacy, I'd rather not spec anything that we don't need to compat / interop.

    
    

    
  
The pull request to revamp fieldset/legend rendering adds mapping to text-align:

https://github.com/whatwg/html/pull/3934/files#diff-36cd38f49b9afa08222c0dc9ebfe35ebR111824

Note that text-align is separate from the legend's position per spec. You can change the position with CSS by using margins, or with the align attribute (which maps to margins). You can change text alignment with CSS using text-align (doesn't change position per spec), or with the align attribute (maps to text-align).

As for why, I saw this as an opportunity to make 'align' slightly more consistent, since we effectively have a 2/2 split on the behavior. But I don't have a strong opinion about this, specifying Gecko seems OK to me. I can ask around to see if others have an opinion on this.
Flags: needinfo?(zcorpan)

    
    

    
  
I must say that I find it strange that Blink / WebKit accept ALIGN on every block-displayed element. Then again, one can say that anything that applies to DIV (the most generic block element that we have, after all) should apply everywhere, so that there's as little magic to DIV as possible.

One observation I just made is that TABLE ALIGN "left" or "right" sets the float property (which I did know from before), but leaves text-align alone (which I didn't know) (all browsers I tested agree on this).

So it doesn't occur as naturally to me as before that ALIGN on a LEGEND *has to* affect text-align.

    
    

    
  
(In reply to Morten Stenshorne from comment #16)
> I must say that I find it strange that Blink / WebKit accept ALIGN on every
> block-displayed element. Then again, one can say that anything that applies
> to DIV (the most generic block element that we have, after all) should apply
> everywhere, so that there's as little magic to DIV as possible.

I don't think we're willing to generalize this mapping to all elements.
The HTML spec and Firefox/Edge is in agreement that it should only be
applied to a handful of elements, and as far as I know that's enough
for web-compat.


> One observation I just made is that TABLE ALIGN "left" or "right" sets the
> float property (which I did know from before), but leaves text-align alone
> (which I didn't know) (all browsers I tested agree on this).
> 
> So it doesn't occur as naturally to me as before that ALIGN on a LEGEND
> *has to* affect text-align.

Right, so I think we should leave out 'text-align' for <legend> if we can.
Firefox/Edge already has that behavior and I haven't seen any web-compat
issues reported about it.  Given that <legend> shrink-wraps by default,
I doubt many pages depend on it either way.

    
    

    
  
(In reply to Mats Palmgren (:mats) from comment #17)
> (In reply to Morten Stenshorne from comment #16)
> > I must say that I find it strange that Blink / WebKit accept ALIGN on every
> > block-displayed element. Then again, one can say that anything that applies
> > to DIV (the most generic block element that we have, after all) should apply
> > everywhere, so that there's as little magic to DIV as possible.
> 
> I don't think we're willing to generalize this mapping to all elements.
> The HTML spec and Firefox/Edge is in agreement that it should only be
> applied to a handful of elements, and as far as I know that's enough
> for web-compat.

Where's ALIGN specified, anyway? I can only find it in old HTML 4 specs.

> > One observation I just made is that TABLE ALIGN "left" or "right" sets the
> > float property (which I did know from before), but leaves text-align alone
> > (which I didn't know) (all browsers I tested agree on this).
> > 
> > So it doesn't occur as naturally to me as before that ALIGN on a LEGEND
> > *has to* affect text-align.
> 
> Right, so I think we should leave out 'text-align' for <legend> if we can.
> Firefox/Edge already has that behavior and I haven't seen any web-compat
> issues reported about it.  Given that <legend> shrink-wraps by default,
> I doubt many pages depend on it either way.

Good points. I'm pretty much convinced now. This makes LEGEND behave more like TABLE, which is okay, since they both kinda shrink-wrap. :)
Priority: -- → P3

    
    

    
  

      
      
      
      

        Attached patch
          
          
        wip (obsolete)
        — 
          — Splinter Review
      

    


  
So I implemented this using "margin-inline-start/end: auto".
It regressed a couple of tests though:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=89204e75d8868a6bcb215e012c20e70771e5efa0&selectedJob=197452607

In short, it changes the layout in a few cases when the fieldset/legend
has different writing-mode/direction.  Simplest case:

  <fieldset dir=ltr>
    <legend dir=rtl>LEGEND</legend>
  </fieldset>

Currently, all UAs renders that legend on the left side, whereas this
patch moves it to the right side.

The root of the problem is that "inline-start" differs from "line-left"
in a few cases:
https://drafts.csswg.org/css-writing-modes/#logical-to-physical

I don't really see a way around that short of introducing new
logical aliases: margin-line-left/right, or changing the spec
and thus accepting the change in layout.

I'm inclined to leave our code as is for now.

    
    

    
  
I guess we could map it with MapAttributesIntoRule but I don't see
a way to get the parent's (fieldset's) writing-mode from there.

    
    

    
  
... but even if we could inject the correct margin-* values from
there it wouldn't be standard CSS so it's not a solution we
could put in the spec anyway.

    
    

    
  
(In reply to Morten Stenshorne from comment #18)
> Where's ALIGN specified, anyway? I can only find it in old HTML 4 specs.

AFAICT, the align attribute on all elements are "Non-conforming features":
https://html.spec.whatwg.org/multipage/obsolete.html#non-conforming-features

The only spec for it is what is specified under Rendering:
https://html.spec.whatwg.org/multipage/#toc-rendering

    
    

    
  
How about if we map it to 'justify-self' instead?
It's NOT inherited (unlike 'text-align') and its semantics matches
the legend alignment within the fieldset perfectly (unlike 'text-align').
"Justifies the box (as the alignment subject) within its containing block
(as the alignment container) along the inline/row/main axis of
the alignment container"
https://drafts.csswg.org/css-align-3/#justify-self-property

Its left/right/center values have the correct meaning AFAICT:
"left: Aligns the alignment subject to be flush with the alignment
container’s line-left..."
https://drafts.csswg.org/css-align-3/#valdef-justify-content-left

It seems like a pretty good fit to me.

    
    

    
  
We could do that, but auto margins still need to work on legend, and it seems justify-self doesn't yet work on general blocks.

    
    

    
  
I have updated the spec in

https://github.com/whatwg/html/pull/3934/commits/4363a9406003c7e525453eaf5e2ddca9b222bd04

and tests in

https://github.com/web-platform-tests/wpt/pull/12811/commits/747fdc35aba44cca1873411a1d9d0176b55d9212

I also removed the rule to let <div align> affect legend position, since that seems annoying to implement.

    
    

    
  


  
This wip patch maps the align=left/right/center to CSS justify-self values.
It also adds support for justify-self(*) in layout for the "rendered legend"
inside a fieldset box.

(*) only left/right/start/end/normal so far, but it should be trivial
to extend that to the full set of justify-self values.

It passes all existing regression tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a7a10f260a6cf6510c4e41a74b68bf7f1b6a9c8&selectedJob=197645176

It seems to me this is the right approach for aligning a legend
box inside its parent.  I'll try it with bug 1483787 next and see
if any issues comes up...
Assignee: nobody → mats

        Attachment #9006391 -
        Attachment is obsolete: true
Summary: <legend align> should map to text-align → <legend align> should map to justify-self

    
    

    
  
The bug assignee is inactive on Bugzilla, so the assignee is being reset.


Assignee: MatsPalmgren_bugz → nobody

    
  
Severity: normal → S3

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: