[webkit-reviews] review granted: [Bug 98876] [Qt] REGRESSION (r130851): fast/text/word-space-with-kerning.html fails : [Attachment 170104] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 98876: [Qt] REGRESSION (r130851): fast/text/word-space-with-kerning.html
fails
https://bugs.webkit.org/show_bug.cgi?id=98876

Attachment 170104: Patch
https://bugs.webkit.org/attachment.cgi?id=170104&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=170104&action=review


>>> Source/WebCore/platform/graphics/qt/FontQt.cpp:216
>>> +	     return width + run.expansion() - m_wordSpacing;
>> 
>> Where do you find that logic in the simple text code path?
> 
> I didn't find it, I just instrumented the code to call both and dumped the
results.
> 
> "A" was 17px wide by both
> " A" was 83px wide by complex, and 23px by simple
> " A " was 149px wide by complex, and 89px by simple.

Indeed, that logic existed before r113968 and in that revision was moved into
FontQt4.cpp, but it's missing here.

I personally would prefer if it was written like before also:

    if (treatAsSpace(run[0]))
	width -= m_wordSpacing

and then share the return:

    return width + run.expansion();

But the change seems correct.