Closed Bug 1155880 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Layout: Tables, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(3 files)

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".
[sorry, had assignee & CC fields' contents swapped when filing]
Assignee: jfkthame → dholbert
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).
Attachment #8594215 - Flags: review?(jfkthame)
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).
Attachment #8594217 - Flags: review?(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.....
Attachment #8594213 - Flags: review?(jfkthame) → review+
Attachment #8594215 - Flags: review?(jfkthame) → review+
Attachment #8594217 - Flags: review?(jfkthame) → review+
(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.)
(Thanks for the reviews!)
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.