411059 - Tab character in table cell with white-space:pre leads to table border overlapping text

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase: table border should not overlap text

A tab character in a table cell can trigger:

WARNING: Tabs encountered in a situation where we don't support tabbing: file /Users/jruderman/trunk/mozilla/layout/generic/nsTextFrameThebes.cpp, line 2293

Also, the border of the table overlaps the table cell, so the table seems to be confused about how long the text is.  In bug 375058, Bernd said he thought this wasn't a table bug.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's hard to determine the preferred or minimum width of text containing a preformatted tab, because the width of the tab depends on where the tab occurs in the line and thus on where line breaks have been placed. This testcase might be fixable but there are more general testcases that aren't. Any thoughts David?

Comment 2

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

I think, just like everything else, we should assume that we take all optional breaks for GetMinWidth, and don't take any optional breaks for GetPrefWidth.  I'm having trouble thinking of what that wouldn't be sufficient for.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I guess you're right, if we use the current width in the InlineData as the x position to figure out where the next tab stop is, we should be OK. Inserting a break before a tab can never make the content wider, and removing a break before a tab can never make the content narrower.

Comment 7

[Elmar Ludwig]

This used to work in Firefox 2, so marking as regression.

Comment 8

[Darrell Kindred]

here's a somewhat more general test case, with tabs at various points within a line:  http://www.cs.cmu.edu/~dkindred/tmp/ff3-width-bug.html

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Adding dependency on bug 230555 since the patch there treads on AddInline(Min/Pref)WidthForFlow which I need to change here.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Includes a reftest partially based on dkindred's testcase.

Comment 12

[Simon Montagu :smontagu]

Comment on attachment 333892 [details] [diff] [review]
fix

>   PRBool collapseWhitespace = !textStyle->WhiteSpaceIsSignificant();
>   PRBool preformatNewlines = textStyle->NewlineIsSignificant();
>+  PRBool preformatTabs = textStyle->WhiteSpaceIsSignificant();

The extra boolean which is just equivalent to !collapseWhitespace seems redundant to me, but OK if you think it's worth it for clarity.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed c4b0381a259b. I think the extra boolean helps clarity significantly.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 333892 [details] [diff] [review]
fix

Pretty bad regression in 1.9, no regressions noted on trunk so far, so we might want to take on 1.9.0.x.

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 333892 [details] [diff] [review]
fix

Looks ugly, but I think we have to minus for the branch. Appeal to mconnor directly if you disagree.