381703 - nsFirstLetterFrame shouldn't compute its size before reflow

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Attachment: testcase

The attached testcase shows a bug where the available width passed to a text frame for text after a first-letter ("ab d") is not being adjusted for the width of the first-letter "a". This leads to the text not wrapping where it should.

The cause is that "ab d" is wrapped in an nsFirstLetterFrame, which had had ComputeSize() called on it, which returned zero for the width. Then when it gets reflowed the reflow state's ComputedWidth() is not NS_UNCONSTRAINEDSIZE so we don't adjust the available width in nsLineLayout::ReflowFrame:
  if (reflowState.ComputedWidth() == NS_UNCONSTRAINEDSIZE)
    reflowState.availableWidth = psd->mRightEdge - psd->mX;

I think the best fix is to make nsFirstLetterFrame behave like inlines and text, overriding ComputeSize to return NS_UNCONSTRAINEDSIZE. I'll attach a patch for that.

It's a bit unfortunate that non-first-letter text is wrapped in nsFirstLetterFrames. I guess it's simpler that way. (The styles are correct because only the true first-letter has the right pseudo-style.)

I don't see an easy way to make a reftest for this...

Comment 1

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

Attachment: fix

Hmm, I wonder if this works with floating first-letters. Maybe ComputeSize should only be overridden for non-first-letter nsFirstLetterFrames.

Comment 2

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

Attachment: better patch

I like this better. True first-letters keep the current behaviour of computing their size in ComputeSize(); non-first-letters behave like spans.

Comment 3

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

Comment on attachment 265794 [details] [diff] [review]
better patch

>+    // We're behaving like a span; don't compute size before reflow.

How about:
  We're wrapping the text *after* the first letter, so behave like an inline frame.
?

r+sr=dbaron

Comment 4

[Martijn Wargers (dead)]

The patch was already landed, so is this bug fixed?

Comment 5

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

yes