Closed Bug 731858 Opened 8 years ago Closed 8 years ago

TextOverflow creates 2 display items for a single frame

Categories

(Core :: Layout: Text and Fonts, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

Attachments

(1 file)

TextOverflow::CreateMarkers can create 2 nsDisplayTextOverflow items for a single frame. The pair of frame pointer and GetPerFrameKey() is meant to be unique for a display item.

I'm surprised that this isn't caught by FrameLayerBuilder currently, but it is with my DLBI code.

Patch adds a unique index to nsDisplayTextOverflow (same as was done for nsDisplayTransform) so that GetPerFrameKey() returns a unique value.
Attachment #601817 - Flags: review?(matspal)
Comment on attachment 601817 [details] [diff] [review]
Add index numbers to nsDisplayTextOverflow

Thanks, looks good, but could you write it like this instead:

  virtual PRUint32 GetPerFrameKey() {
    return (mIndex << TYPE_BITS) | nsDisplayItem::GetPerFrameKey();
  }

so that it fits in < 80 columns.

FYI, the function ClipMarker() potentially also creates a nsDisplayClip per
nsDisplayTextOverflowMarker item (wrapping it), I assume that's OK since
nsDisplayClip::GetPerFrameKey() returns zero.
Attachment #601817 - Flags: review?(matspal) → review+
(In reply to Mats Palmgren [:mats] from comment #1)
> Comment on attachment 601817 [details] [diff] [review]
> Add index numbers to nsDisplayTextOverflow
> 
> Thanks, looks good, but could you write it like this instead:
> 
>   virtual PRUint32 GetPerFrameKey() {
>     return (mIndex << TYPE_BITS) | nsDisplayItem::GetPerFrameKey();
>   }
> 
> so that it fits in < 80 columns.

Sure, done.

> 
> FYI, the function ClipMarker() potentially also creates a nsDisplayClip per
> nsDisplayTextOverflowMarker item (wrapping it), I assume that's OK since
> nsDisplayClip::GetPerFrameKey() returns zero.

Yeah, this is fine, nsDisplayClip isn't stored in FrameLayerBuilder.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4392f9d56d2e
Assignee: nobody → matt.woodrow
https://hg.mozilla.org/mozilla-central/rev/4392f9d56d2e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.