Closed Bug 1177606 Opened 4 years ago Closed 4 years ago

table contents misplaced in vertical-rl mode when the table has an explicitly specified width

Categories

(Core :: Layout: Tables, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Attached file table2.html
See attached testcase; when we *do* know the width of the table, we get the placement of the table body all wrong. :(
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8626397 [details] [diff] [review]
Fix containerWidth usage for vertical-rl tables where the table width is explicitly set

>+++ b/layout/tables/nsTableFrame.cpp
> void
> nsTableFrame::DistributeBSizeToRows(const nsHTMLReflowState& aReflowState,
>                                     nscoord                  aAmount)
> {
>   WritingMode wm = aReflowState.GetWritingMode();
>   LogicalMargin borderPadding = GetChildAreaOffset(wm, &aReflowState);
> 
>+  nscoord containerWidth = aReflowState.ComputedWidth();
>+  if (containerWidth == NS_UNCONSTRAINEDSIZE) {
>+    containerWidth = 0;
>+  } else {
>+    containerWidth += aReflowState.ComputedPhysicalBorderPadding().LeftRight();
>+  }

Per IRL discussion, could you file a followup on abstracting this ^ logic into a method on nsLayoutUtils or somewhere?

>-      // Make sure child views are properly positioned
>+
>+      if (wm.IsVerticalRL()) {
>+        nscoord rgWidth = rgFrame->GetRect().width;
>+        for (nsTableRowFrame* rowFrame = rgFrame->GetFirstRow();
>+             rowFrame; rowFrame = rowFrame->GetNextRow()) {
[etc]

This new chunk needs an explanatory comment.

r=me with that.
Attachment #8626397 - Flags: review?(dholbert) → review+
(Also: please include a reftest.)
Flags: in-testsuite?
I'm not sure if it's this bug or a different one, but the table body also gets displaced when <td>s have padding on the vertical sides.
(In reply to Simon Montagu :smontagu from comment #4)
> I'm not sure if it's this bug or a different one, but the table body also
> gets displaced when <td>s have padding on the vertical sides.

Might be related to this or to bug 1177600. In my local build with both that patch and this one, adding padding to the <td>s seems to work properly.
After working with more different test cases, "when <td>s have padding on the vertical sides" might not be the exact criterion. Table content gets displaced in tables with vertical-rl from time to time, and it isn't always clear what the deciding factor is. If I still see it happening after this is checked in I'll pursue it further.
Here are a couple of reftests where an explicit width is specified on the table; the vertical-rl case fails until bug 1177600 and this bug are fixed.
https://hg.mozilla.org/mozilla-central/rev/645991c76862
https://hg.mozilla.org/mozilla-central/rev/30031b730aa2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.