1155880 - Rename table layout strategy "column width" APIs to use "isize" instead

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Daniel Holbert [:dholbert]]

Lots of our table code is about determining column widths. In particular, the nsITableLayoutStrategy interface does most of its interesting work in the function "ComputeColumnWidths".  (and the implementations of this method call nsTableColFrame::GetFinalWidth/SetFinalWidth)

In vertical writing-modes, though, column direction will be swapped (so that each column will be a horizontal thing, instead of a vertical thing).  Then, we'll need to be computing & setting column *heights*, not *widths*.

In general, we should be computing the inline-dimension size of table columns; so we should generify this language to just use "ISize" instead of "Width".

Comment 1

[Daniel Holbert [:dholbert]]

[sorry, had assignee & CC fields' contents swapped when filing]

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: Rename nsITableLayoutStrategy::ComputeColumnWidths and associated comments to use "ISize" instead of "Width"

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: Rename nsTableColFrame Get/Set/ResetFinalWidth methods to use "ISize" instead of "Width"

Following the rabbit-hole -- the function changed in part 1 (ComputeColumnWidths) does its work using nsTableColFrame::GetFinalWidth/SetFinalWidth.

So, this patch does the same renaming on these nsTableColFrame methods (and the member-var "mFinalWidth" controlled by these methods, since its usages are pretty minimal & simple).

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: Rename nsTableFrame::GetColumnWidth to use "ISize" instead of "Width"

Following the rabbit-hole one more step: as shown in previous patch, nsTableColFrame::GetFinalWidth (now called GetFinalISize) is also exposed via a nsTableFrame method, "GetColumnWidth".

This patch renames that method as well, for consistency / completeness (or an approximation thereof).

Comment 5

[Jonathan Kew [:jfkthame]]

The renaming here all looks reasonable, though it puts us in a state where various "ISize" methods are in fact still working on physical-coordinate widths, so the new names are rather misleading at this stage. But you're intending to fix that in following patches/bugs, I'm sure.....

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> The renaming here all looks reasonable, though it puts us in a state where
> various "ISize" methods are in fact still working on physical-coordinate
> widths, so the new names are rather misleading at this stage.

True -- though, these classes are already in that state (& inconsistently so). These TableLayoutStrategy classes already have methods GetPrefISize() and MarkIntrinsicISizesDirty(), which actually operate on width-flavored values.  These methods were renamed last year in bug 1031241 (parts 1 and 3).

So, the patches here just make this code more consistent about displaying an "ISize" facade. :)

> But you're
> intending to fix that in following patches/bugs, I'm sure.....

At some point soon, yes. (not necessarily immediately, but sometime this quarter, in support of bug 	1077521.)

Comment 7

[Daniel Holbert [:dholbert]]

(Thanks for the reviews!)

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/352e8f8c5915
https://hg.mozilla.org/integration/mozilla-inbound/rev/15f970a06d93
https://hg.mozilla.org/integration/mozilla-inbound/rev/98fd24ddd601

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/352e8f8c5915
https://hg.mozilla.org/mozilla-central/rev/15f970a06d93
https://hg.mozilla.org/mozilla-central/rev/98fd24ddd601