Bug 177493 – Extract out combined text query into a member function

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Daniel Bates]

Extract out combined text query into a member function.

[Daniel Bates]

Created attachment 321844 [details]
Patch

[Daniel Bates]

Created attachment 321848 [details]
Patch

[zalan]

Comment on attachment 321848 [details]
Patch

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

> Source/WebCore/rendering/InlineTextBox.cpp:1117
> +    return lineStyle().hasTextCombine() && is<RenderCombineText>(renderer()) && downcast<RenderCombineText>(renderer()).isCombined() ? &downcast<RenderCombineText>(renderer()) : nullptr;

Not sure why we check both the style and the type of the renderer here. If the style has text combine, the renderer is supposed to be RenderCombineText (and vice versa). Also do you mind checking if fontToUse() could take advantage of this change -it's a static function though).

[Daniel Bates]

(In reply to zalan from comment #3)
> Comment on attachment 321848 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321848&action=review
> 
> > Source/WebCore/rendering/InlineTextBox.cpp:1117
> > +    return lineStyle().hasTextCombine() && is<RenderCombineText>(renderer()) && downcast<RenderCombineText>(renderer()).isCombined() ? &downcast<RenderCombineText>(renderer()) : nullptr;
> 
> Not sure why we check both the style and the type of the renderer here. If
> the style has text combine, the renderer is supposed to be RenderCombineText
> (and vice versa). 

This was strange to me as well when I was performing the refactor. This patch is just moving code. I hope you do not mind that defer this investigation to another bug.

> Also do you mind checking if fontToUse() could take advantage of this change -it's a static function though).

Yes, I plan to update fontToUse() to take advantage of this change. I plan to do this in bug #177495.

[Daniel Bates]

Comment on attachment 321848 [details]
Patch

Clearing flags on attachment: 321848

Committed r222528: <http://trac.webkit.org/changeset/222528>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34692884>