Bug 96890

Summary: REGRESSION (r126763): css1/pseudo/firstline.html fails when using the complex text code path
Product: WebKit Reporter: mitz
Component: TextAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: eric, ned, webkit.review.bot
Priority: P1 Keywords: Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed fix (no change log or test yet)
none
Invalidate the TextLayout on font change eric: review+

Description mitz 2012-09-16 13:47:09 PDT
To reproduce:
1. run-webkit-tests --complex css1/pseudo/firstline.html
—or—
1. Open the test in Safari
2. Select Always Use Complex Text Code Path in Debug > Miscellaneous
3. Reload

This was caused by <http://trac.webkit.org/r126763>, the fix for bug 83045.
Comment 1 mitz 2012-09-16 14:03:45 PDT
This happens because the same RenderText is rendered with two different fonts on different lines. I think the solution is to add a Font* to RenderTextInfo.
Comment 2 mitz 2012-09-16 14:13:52 PDT
Created attachment 164324 [details]
Proposed fix (no change log or test yet)
Comment 3 mitz 2012-09-16 16:09:04 PDT
Created attachment 164325 [details]
Invalidate the TextLayout on font change
Comment 4 Eric Seidel (no email) 2012-09-16 16:36:14 PDT
Comment on attachment 164325 [details]
Invalidate the TextLayout on font change

View in context: https://bugs.webkit.org/attachment.cgi?id=164325&action=review

> Source/WebCore/ChangeLog:10
> +        When a first-line style specifies a font, different pieces of the same RednerText can be

RenderText

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2440
> +                renderTextInfo.m_font = &f;

Do we ever need to clear this pointer?
Comment 5 Eric Seidel (no email) 2012-09-16 16:36:32 PDT
Comment on attachment 164325 [details]
Invalidate the TextLayout on font change

Sorry, didn't mean to clear Sam's review.
Comment 6 mitz 2012-09-16 16:39:37 PDT
Fixed in <http://trac.webkit.org/r128713>.

(In reply to comment #4)
> (From update of attachment 164325 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164325&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        When a first-line style specifies a font, different pieces of the same RednerText can be
> 
> RenderText

Fixed in <http://trac.webkit.org/r128714>.

> 
> > Source/WebCore/rendering/RenderBlockLineLayout.cpp:2440
> > +                renderTextInfo.m_font = &f;
> 
> Do we ever need to clear this pointer?

No, the RenderTextInfo gets destroyed at the end of line layout.