Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060309 Firefox/1.6a1 If a table is too narrow for its contents and it has " <br>" (i.e. space <br>) in it, there will be two linebreaks instead of one. Testcase is in URL field. It seems to only affect the widest cell, even if other cells are too narrow for their contents as well. This is a regression. It worked fine in 2006-02-16 builds, it fails in 2006-03-09. I'll narrow it down farther.

Regressed between 2006-03-07 and 2006-03-08 Only thing that looks like it could have caused it is checkin for bug 164700

I think this 'regressed' behavior is correct behavior... I think that if we want to fix this, we need to trim the spaces around the <br>. # that's easy. But that is not specified on HTML spec...

*** Bug 331042 has been marked as a duplicate of this bug. ***

Comment on attachment 214684 [details] [diff] [review] quirk patch rv1.0 I found a bug in this patch...

"we shoud not flag a break-after status if next frame is nsBRFrame." I agree with this general idea. However, I was wondering if we cannot resolve the problem in another simpler way, by simply distinguishing the hard-break of <br> vs. the "soft-break" (that we are compelled to have to avoid the letter-spacing bug). That is, in nsTextFrame, simply add: + else if (aTextData.mTrailingSpaceTrimmed && rs == NS_FRAME_COMPLETE) { + rs = NS_INLINE_LINE_BREAK_AFTER(rs); + lineLayout.SetLineEndsInSoftBR(PR_TRUE); + } Then, in nsBRFrame, add: +if (!ll->GetLineEndsInSoftBR()) { aStatus = NS_INLINE_BREAK | NS_INLINE_BREAK_AFTER | NS_INLINE_MAKE_BREAK_TYPE(breakType); +else { // promote the existing soft-break to a hard-break -- (the main idea) + aStatus = NS_FRAME_COMPLETE; + ll->SetLineEndsInSoftBR(PR_FALSE); +} ll->SetLineEndsInBR(PR_TRUE); (with of course the Get/SetLineEndsInSoftBR flag added in linelayout) What do you think?

I create the patch from your idea. But it's not working fine with the case of bug 331042 that is having white-space only frame between current frame and br frame...

This patch works fine, but this is not simple...

Here is a possible simpler patch that does the trick for me. The idea is to effectively break only if linelayout attempts to put a word (rather than break rightway, assuming that linelayout might add another word on the line, which it might not). BTW, did you checkin the intended patch for bug 164700? With my tree, I am not seeing any word-spacing between the various "EntryX EntryY" (except before Entry4) on this testcase: https://bugzilla.mozilla.org/attachment.cgi?id=114531

Good! It's clever patch! But that has a mistake, we need to check the text frame has only white spaces and they are skipped. Who can review this? - else if (lineLayout.GetLineEndsInSoftBR()) { + else if (lineLayout.GetLineEndsInSoftBR() && column != mColumn) {

(In reply to comment #12) > BTW, did you checkin the intended patch for bug 164700? With my tree, I am not > seeing any word-spacing between the various "EntryX EntryY" (except before > Entry4) on this testcase: > https://bugzilla.mozilla.org/attachment.cgi?id=114531 > I checked-in following patch. http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTextFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/generic&command=DIFF_FRAMESET&rev1=1.548&rev2=1.549 Do you mean that the word-spacing is not applied on your build?

We don't need to chase after the flag to reset it. The linebreak causes a new line and this is handled with a new linelayout object in a clean state. Trying bz for the review.

Here is what I see for attachment 114531 [details] now. It is different from what I used to see (attachment 186900 [details], modulo the linebreak bug).

Also, attachment 186900 [details] was using word-spacing: 100pt; instead of the present 6pt.

I can see that your tree fails |GetTextDimensionsOrLength|.

After the VC8 Express problem (bug 329209), it is strange that I am having this other text-measuring problem and that you are okay. I have rebuilt my tree with "ac_add_options --enable-default-toolkit=windows". This re-enables the native GDI instead of cairo-windows, which is now the default on Windows. Guess what? With that build option, the spacing comes back as expected. Are you not using the default cairo-windows (and VC8)?

Currently, thebes doesn't support letter-spacing, word-spacing and justify. See bug 327184. We should use non-cairo build for working on text rendering, now.

Comment on attachment 215642 [details] [diff] [review] even simpler r+sr=bzbarsky if you document in nsLineLayout what this flag means and how it differs from LL_LINEENDSINBR.

Checked in the trunk.