776443 - calc() doesn't work on table cells or rows

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[j.j.]

Attachment: testcase

calc() doesn't work on table cells.

Testcase works partly in IE and WebKit (width part fails).

Comment 1

[Boris Zbarsky [:bzbarsky]]

What the spec actually says seems to be this:

  Given the complexities of width and height calculations on table cells and table
  elements, math expressions involving percentages for widths and heights on table
  columns, table column groups, table rows, table row groups, and table cells in 
  both auto and fixed layout tables MAY be treated as if ‘auto’ had been specified. 

so the width behavior you see is perfectly fine per spec.

That said, we actually do this for _any_ calc expression, not just ones involving percentages.....

Comment 2

[Boris Zbarsky [:bzbarsky]]

There are two places in the reflow state that treat calc() as auto for tables, and also various places in the table code itself.

David, how do you feel about making calc() not involving percentages just work here?

Comment 3

[j.j.]

anyways, needs documentation

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> There are two places in the reflow state that treat calc() as auto for
> tables, and also various places in the table code itself.
> 
> David, how do you feel about making calc() not involving percentages just
> work here?

That's fine; probably what I should have done originally.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And nsStyleCoord::ConvertsToLength() is a convenient helper for this type of check.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Support percent-less calc for internal table element heights.

Any ideas on how to test some of those codepaths are much appreciated!

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Support percent-less calc for internal table element heights.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Support percent-less calc for internal table element heights.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Support percent-less calc for internal table element widths.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 645030 [details] [diff] [review]
part 1.  Support percent-less calc for internal table element heights.

r=dbaron

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 645040 [details] [diff] [review]
part 2.  Support percent-less calc for internal table element widths.

It would be good to also change nsLayoutUtils::IntrinsicForContainer to check !styleWidth.ConvertsToLength instead of checking styleWidth.GetUnit() != eStyleUnit_Coord for the codepath that avoids doing intrinsic width computation because we know GetAbsoluteCoord(styleWidth, w) is going to succeed.  (Though the reason it's skipping the intrinsic width computation should probably also be explained in the comment.)  (I noticed this because of the codepaths leading from FixedTableLayoutStrategy to IntrinsicForContainer that you're now going to call in those cases because of this patch.)

r=dbaron with that

Comment 13

[Boris Zbarsky [:bzbarsky]]

Done, and:

   https://hg.mozilla.org/integration/mozilla-inbound/rev/e02a02fd1208
   https://hg.mozilla.org/integration/mozilla-inbound/rev/1a8b11ae0f4b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Sorry, something in your push turned the mochitests orange. I had to back the entire thing out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/461a2225b7ba

Comment 15

[Boris Zbarsky [:bzbarsky]]

The orange was from bug 812392.  But this push did cause reftest failures.  Fixed those, and repushed:

   https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0893a44057
   https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8196bc19b4

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7f0893a44057
https://hg.mozilla.org/mozilla-central/rev/cf8196bc19b4