nsFirstLetterFrame shouldn't compute its size before reflow

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 265791 [details]
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...
Created attachment 265793 [details] [diff] [review]
fix

Hmm, I wonder if this works with floating first-letters. Maybe ComputeSize should only be overridden for non-first-letter nsFirstLetterFrames.
Created attachment 265794 [details] [diff] [review]
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.
Attachment #265793 - Attachment is obsolete: true
Attachment #265794 - Flags: superreview?(dbaron)
Attachment #265794 - Flags: review?(dbaron)
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
Attachment #265794 - Flags: superreview?(dbaron)
Attachment #265794 - Flags: superreview+
Attachment #265794 - Flags: review?(dbaron)
Attachment #265794 - Flags: review+

Updated

10 years ago
Blocks: 368912
The patch was already landed, so is this bug fixed?
yes
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.