351641 - [reflow branch] Long strings of Japanese text don't wrap in table-cells

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[philippe (part-time)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060907 Camino/1.2+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060831 Minefield/3.0a1 ID:000000000

When a table-cell contains long strings of Japanese text, the text does not wrap and the table(-cell) expands to fit the length of the text.

Probably happens with other CJK languages, I haven't tested.

Reproducible: Always

Steps to Reproduce:
1. See test case
2.
3.



Expected Results:  
The text wraps according to the width of the table-cell

Comment 1

[philippe (part-time)]

Attachment: test case

2 tables, width should be equal.

Comment 2

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

So I sort of botched nsTextFrame::GetMinWidth on the reflow branch, but I'm really not sure I understand how to do it right.  The current code has this whole mess of "WordFrames" on nsLineLayout to deal with words that span across multiple frames, working around GetNextWord only looking within the same frame.

Basically, I need to figure out when I want to continue accumulating min-width and when I want to consider something a possible break.  It seems like all GetNextWord boundaries within a frame are possible breaks, but **some** GetNextWord boundaries at frame edges are not possible breaks.  roc, you don't happen to know how I tell which those are?

Comment 3

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

I'm confused. In the testcase, each run that we want to break within has just one textframe, I think, so why have you identified frame-spanning words as the issue?

Comment 4

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

Because my current code is designed so that it works for "this isavery<span>long</span>word", and therefore breaks in this case.  And I'm not sure how to fix this without breaking that one.

Comment 5

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

(In other words, my current code assumes that the only breakpoints are at whitespace, which is definitely wrong.  But I don't know how to remove that assumption without adding the assumption that any frame boundary is an allowed break point, because I don't see where we decide that some frame boundaries are allowed break points and some are not.)

Comment 6

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

GetNextWord() will stop at either a line break or the end of the content node. It doesn't respect the current frame content length (in fact, that is not made available to nsTextTransformer). The way regular reflow handles line breaks between nodes is that when a text frame ends in non-whitespace we look ahead through subsequent text nodes calling nsILineBreaker::BreakInBetween at the junctions between frames.

I suppose you could emulate this by calling FindNextText and BreakInBetween to see whether a break is allowed.

My patch in bug 343445 changes this, and the new textframe code changes it some more. In bug 343445 I remove FindNextText, instead I just keep track of the last text frame and do BreakInBetween for the last text frame and the current text frame to see whether to allow a break before the current text frame. It does not do GetMinWidth/MEW computation correctly, though, as I noted in the bug.

The new text frame code builds on the patch in bug 343445 and also doesn't do GetMinWidth/MEW computation correctly.

The way I would like to do GetMinWidth on reflow branch is that when the frame ends with a word, we always retain the current word width in the state. We also keep track of the last text frame in the state just like bug 343445 does in linelayout. Then in the next text frame, we do a CanBreakInBetween check, and if a break is allowed, flush the current word width to the min-width before proceeding with that text frame. Of course you also need to flush at the end of the run of inlines.

I'm not sure how to proceed here. I'm tempted to say "let's just break MEW/min-width computation so that it doesn't look across node boundaries, for now" (i.e., accept the patch in bug 343445), then reflow branch can land without changing behaviour in that regard, and so can my textframe work. Then once all these pieces have landed, I should be able to fix things up without much trouble. I don't want to try to do correct cross-node MEW computation for bug 343445 in non-reflow-branch, but I also don't want to wait for the reflow branch before I get the new text frame in the tree (albeit not turned on); getting it in the tree really requires the MEW-breaking linelayout changes from bug 343445. I could do my work on the reflow branch itself but then I'd get out of sync with the Thebes font changes that are happening on trunk! What a mess!

Comment 7

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

The code that breaks nonwhitespace text runs at node boundaries is here, BTW:
http://lxr.mozilla.org/mozilla/source/layout/generic/nsTextFrame.cpp#6414

Comment 8

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

I just fixed this on the reflow branch, although I there's actually also a bug in the table sizing behavior, which I think I need to fix in BasicTableLayoutStrategy::ComputeColumnWidths .