<legend align> should map to justify-self

NEW
Assigned to

Status

()

defect
P3
normal
9 months ago
8 months ago

People

(Reporter: zcorpan, Assigned: mats)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 affected)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

9 months ago
See https://github.com/web-platform-tests/wpt/pull/12811

In Firefox text-align is always "start".
Reporter

Updated

9 months ago
Blocks: fieldset
Assignee

Comment 1

9 months ago
Posted file Testcase #1
Assignee

Comment 2

9 months ago
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)

Comment 3

9 months ago
<!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.
Assignee

Comment 6

9 months ago
Posted 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').
Assignee

Comment 7

9 months ago
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?
Assignee

Comment 10

9 months ago
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.
Assignee

Comment 12

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

Comment 13

9 months ago
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.
Reporter

Comment 15

9 months ago
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)

Comment 16

9 months ago
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.
Assignee

Comment 17

9 months ago
(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.

Comment 18

9 months ago
(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
Assignee

Comment 19

9 months ago
Posted 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.
Assignee

Comment 20

9 months ago
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.
Assignee

Comment 21

9 months ago
... 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.
Assignee

Comment 22

9 months ago
(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
Assignee

Comment 23

9 months ago
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.
Reporter

Comment 24

9 months ago
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.
Reporter

Comment 25

9 months ago
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.
Assignee

Comment 26

9 months ago
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
Assignee

Updated

8 months ago
Summary: <legend align> should map to text-align → <legend align> should map to justify-self
You need to log in before you can comment on or make changes to this bug.