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

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: table2.html

See attached testcase; when we *do* know the width of the table, we get the placement of the table body all wrong. :(

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Fix containerWidth usage for vertical-rl tables where the table width is explicitly set

Comment 2

[Daniel Holbert [:dholbert]]

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.

Comment 3

[Daniel Holbert [:dholbert]]

(Also: please include a reftest.)

Comment 4

[Simon Montagu :smontagu]

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.

Comment 5

[Jonathan Kew [:jfkthame]]

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

Comment 6

[Simon Montagu :smontagu]

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.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Reftests for vertical tables with specified width

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.

Comment 8

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/645991c76862
https://hg.mozilla.org/integration/mozilla-inbound/rev/30031b730aa2

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/645991c76862
https://hg.mozilla.org/mozilla-central/rev/30031b730aa2