Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
emilio opened this issue Oct 23, 2019 · 17 comments
Open

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

emilio opened this issue Oct 23, 2019 · 17 comments
Labels

Comments

@emilio
Copy link
Collaborator

emilio commented Oct 23, 2019

See https://bugzilla.mozilla.org/show_bug.cgi?id=1590837 / jquery/jquery#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
Copy link
Contributor

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
Copy link
Collaborator Author

emilio commented Oct 28, 2019

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

@Loirooriol
Copy link
Contributor

Loirooriol commented Oct 28, 2019

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).

mgol added a commit to mgol/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Ref jquery#4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
mgol added a commit to mgol/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Ref jquery#4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
mgol added a commit to jquery/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Closes gh-4537
Ref #4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444
mgol added a commit to jquery/jquery that referenced this issue Oct 28, 2019
… in Firefox

Firefox 70 & newer fail this test but the issue there is more profound - Firefox
doesn't subtract borders from table row computed widths.

Closes gh-4537
Ref #4529
Ref https://bugzilla.mozilla.org/show_bug.cgi?id=1590837
Ref w3c/csswg-drafts#4444

(cherry picked from commit c79e1d5)
@FremyCompany
Copy link
Contributor

FremyCompany commented Oct 30, 2019

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:

  • border-*-width should return 10px because border-width is not a property that returns its used value for getComputedStyle;
  • height however should return its used value. So basically, you want to return 42px here, because the used value of border is 0px, and the full row height is 42px so 42px - 0px = 42px.
  • I would thus argue Firefox 70 is correct. The problem is that this kind of correct is also unpractical for authors, because they will inevitably try to compute the full height by adding the border-width and the height, unknowing of the fact that the border was actually ignored in the first place. Chrome avoids that pitfall by subtracting from 42px the 2*10px from the border they report but actually do not use.

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
Copy link
Contributor

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Contributor

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

@emilio
Copy link
Collaborator Author

emilio commented Oct 31, 2019

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
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

atotic commented Dec 18, 2019

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
Copy link
Member

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//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
Copy link

mgol commented Mar 29, 2024

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
Copy link
Contributor

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
Copy link
Contributor

atotic commented Mar 31, 2024

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

I agree with the round-trip idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants