w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.42k stars 648 forks source link

[cssom] Table row resolved width and height. #4444

Open emilio opened 4 years ago

emilio commented 4 years ago

See https://bugzilla.mozilla.org/show_bug.cgi?id=1590837 / https://github.com/jquery/jquery/issues/4529.

In the following test-case:

<table style="border-collapse: separate; border-spacing: 0; background: #baffc9"><tbody>
    <tr id="tr" style="margin: 0; border: 10px solid black; padding: 0">
        <td style="margin: 0; border: 0; padding: 0; height: 42px; width: 42px;"></td>
    </tr>
</tbody></table>
    <div id="result"></div>
<script>
var result = document.getElementById('result');
var tr = document.getElementById('tr');
result.innerHTML = getComputedStyle(tr).width;
</script>

Gecko reports 42px. Blink/WebKit report 22px. EdgeHTML reported auto.

width and height on table rows have no effect, so I think Edge was right on this one, and table rows should behave the same way non-replaced inlines behave, and just return the computed value... Thoughts?

I should update the spec to better reflect implementations on this regard, btw...

Loirooriol commented 4 years ago

width and height on table rows have no effect

Are you sure? From https://drafts.csswg.org/css2/tables.html#height-layout

it is the maximum of the row's computed 'height', [...]

If I set height: 50px to the row, the table grows from 42px to 50px.

width seems to have no effect indeed for horizontal writing modes. But if I set writing-mode: vertical-lr to the table, then width has effect (and height doesn't).

emilio commented 4 years ago

Ah, yeah, good point, I guess this is only about inline-size.

Loirooriol commented 4 years ago

It's not exactly inline-size either, because if you use writing-mode: vertical-lr to the row, then inline-size will refer to the height and it will have effect. So the size that doesn't have effect is the one in the axis of the table's inline size (or the tbody's? Orthogonal flows in table parts are confusing for me).

FremyCompany commented 4 years ago

I see that this is open for discussion later today. I should be able to make the call, but even if not here are a few thoughts:

So, well, this isn't great, but I don't think there is any behaviour here that makes a lot of sense. Firefox 70 is compliant with the specs as they are today, but that behavior is annoying for web developers because unpredictable. Ideally, all browsers would adopt Firefox 68's behaviour (then we need to change CSSOM to enable reporting the used value of border-width on elements which cannot draw borders) but otherwise we might have to specify Chrome's behavior (height return its used value minus the computed value of border-*-width, regardless of the used value of border-*-width).

A final option would be to make a change to css-tables and specify that the computed value of border-*-width on table-track elements in separate mode is forced to 0px. That would make Firefox 68's behavior valid per spec again, for different reasons.

The main concern I have with Chrome's behavior is that it doesn't round-trip. As mentioned before, setting height on a table row does have an effect, and that effect ignores any border-width set on the table row, so basically setting height to Chrome's getComputedStyle().height value can alter the table layout in the end.

FremyCompany commented 4 years ago

While EdgeHTML's behavior makes the most sense in the end, it's also not spec-compatible right now because height should return its used value, not its computed value. The reason Edge reports its computed value is that the row doesn't have a box, so it behaves similarly to a display:none element.

fantasai commented 4 years ago

@Loirooriol The writing-mode property doesn't apply to internal table or ruby elements: they all use the writing-mode of the table wrapper/ruby container box. ( But see also https://www.w3.org/TR/css-writing-modes-3/#placement )

fantasai commented 4 years ago

Also, given @FremyCompany’s observation that Chrome's value doesn't round-trip, I think we really really really should not go with that option. Roundtripping getComputedStyle() values is probably the most fundamental rule about defining them.

Loirooriol commented 4 years ago

@fantasai So you can't use writing-mode to have an orthogonal flow in a table row, but you can use it to change the inline-size mapping with respect to the table, right?

Regarding round-tripping being "the most fundamental rule", note we have precedents that don't round-trip: grid-template-rows and grid-template-columns (the serialization includes leading implicit tracks, but they can only set explicit tracks).

FremyCompany commented 4 years ago

@Loirooriol I woudln't think so. The property just doesn't apply at all.

emilio commented 4 years ago

While EdgeHTML's behavior makes the most sense in the end, it's also not spec-compatible right now because height should return its used value, not its computed value. The reason Edge reports its computed value is that the row doesn't have a box, so it behaves similarly to a display:none element.

Why is it not spec-compatible? https://drafts.csswg.org/cssom/#resolved-values gates the used value condition to "the property applies to the element". That is of course very handwavy, see #3678, but in this case it seems that could apply and the value returned would be the computed one (that is, auto).

MatsPalmgren commented 4 years ago

So if you set writing-mode:vertical-lr on the table then width would return a definite value (the row's block-axis size) and height would return auto I guess?

FremyCompany commented 4 years ago

While EdgeHTML's behavior makes the most sense in the end, it's also not spec-compatible right now because height should return its used value, not its computed value. The reason Edge reports its computed value is that the row doesn't have a box, so it behaves similarly to a display:none element.

Why is it not spec-compatible? https://drafts.csswg.org/cssom/#resolved-values gates the used value condition to "the property applies to the element". That is of course very handwavy, see #3678, but in this case it seems that could apply and the value returned would be the computed one (that is, auto).

The height property does apply on table-rows.

atotic commented 4 years ago

I agree that FF way is correct. Round-tripping height should work.

Looking at the existing Chrome implementation, I've discovered that Chrome's implementation of getComputedStyle().height depends on box-sizing. In the original example, adding this css * { box-sizing: border-box; } makes both FF and Chrome return the same value.

css-meeting-bot commented 4 years ago

The CSS Working Group just discussed [cssom] Table row resolved width and height.

The full IRC log of that discussion <dael> Topic: [cssom] Table row resolved width and height
<dael> github: https://github.com/w3c/csswg-drafts/issues/4444
<dael> emilio: Height doens't apply to table rows depending on writing modes. There was interop issue found by jquery folks. A bunch of discussion on the issue.
<dael> emilio: Various options. Nice to get an agreement.
<dael> emilio: Webkit and Chrome behavior doesn't roundtrip. I propose to return computed value which is what we do for other elements that don't support height. People argued against that
<dael> TabAtkins: I'll proxy Alex and say FF is correct and roundtrip should work. Let's roll
<dael> astearns: Reading thread. What needs to be done to roundtrip height?
<dael> emilio: Using the computed value works. FF does give a resolved value it just roundtrips so not what I proposed.
<dael> emilio: Edge used to report auto
<dael> emilio: And the computed value
<dael> emilio: Computed value of auto FF gets a roundtrip height and Blink doesn't make sense. Edge returned computed value which is consistent with inlines
<dael> astearns: Prop: Return the computed value of height for table rows
<dael> emilio: When it doesn't apply which is not always. Depends on writing mode
<dael> emilio: I think that makes most sense. Alex argued for FF behavior. There are other comments from fremy and oriol saying computed value may not be most useful
<dael> emilio: A bunch of use cases for this
<dael> fantasai: Seems there are two discussions. One is which property applies and therfore has meaningful value. And which is in height axis of table. Then should prop of block axis return auto or used value.
<dael> fantasai: Mapping question should be switching based on writing mode of table. In terms of if we return used value or auto I don't have option other than it hsould round trip
<dael> fantasai: Means block axis on row needs to return. Can't default to auto. Inline axis doesn't matter
<dael> emilio: Used to have stronger opinion but I'm happy to keep discussing and figure out best compromise. If people don't have strong opinion
<dael> astearns: Leave that intent is to have round trip for block axis and leave it at that until we have actual text?
<dael> emilio: Fair
<dael> astearns: Concerns with having round trip value for block axis of table rows?
<dael> astearns: Let's work in issue
mgol commented 5 months ago

Is there any hope at reviving the discussion here? Currently, jQuery penalizes Firefox by relying on offsetHeight which results in non-fractional values for .height(). We'd likely align with the spec and penalize non-compliant browsers instead, but I'd hate to incur a breaking change only to change it back shortly afterwards when a decision to change the spec is made.

FremyCompany commented 4 months ago

Hi! I think we'd need to have someone from Chromium on-board to have this discussion. I'm not sure who owns table layout these days, maybe @atotic knows?

atotic commented 4 months ago

@bfgeek took over tables over from me. He'd know who to talk to for sure.

I agree with the round-trip idea.