Bug 98876

An other bot that stores results for longer time: http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r130851%20%2843632%29/results.html

Skipped on Qt in https://trac.webkit.org/changeset/130873. Please unskip it with the proper fix.

It seems the problem is that our version of Font::floatWidthForComplexText, when used on a string such as " b ", applies word-spacing both before and after the word, while the layout assumes we only apply word-spacing once.

Created attachment 170104 [details] Patch

Btw, quick note. The problem is only related to kerning because kerning enables complex font-path. The problem happens in many more similar cases if we disable simple font-path completely and always choose complex. This means more tests are likely to have been influence by this bug if they caused complex font-path due to some other mechanism.

Comment on attachment 170104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170104&action=review > Source/WebCore/platform/graphics/qt/FontQt.cpp:216 > + // RenderBlockLineLayout expects us to only add word-spacing for trailing spaces, not for leading spaces. > + if (treatAsSpace(run[0])) > + return width + run.expansion() - m_wordSpacing; Where do you find that logic in the simple text code path?

(In reply to comment #6) > (From update of attachment 170104 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=170104&action=review > > > Source/WebCore/platform/graphics/qt/FontQt.cpp:216 > > + // RenderBlockLineLayout expects us to only add word-spacing for trailing spaces, not for leading spaces. > > + if (treatAsSpace(run[0])) > > + 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.

Comment on attachment 170104 [details] Patch 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.

Committed r132205: <http://trac.webkit.org/changeset/132205>