Measuring the width of a space character is redundant
Created attachment 252486 [details] Patch
Comment on attachment 252486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252486&action=review > Source/WebCore/rendering/RenderBlockFlow.cpp:3994 > const UChar space = ' '; Unused.
Created attachment 252497 [details] Patch for landing
Comment on attachment 252497 [details] Patch for landing Attachment 252497 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5588724582711296 New failing tests: fast/ruby/ruby-expansion-cjk-3.html fast/ruby/ruby-expansion-cjk.html http/tests/webfont/fallback-font-while-loading.html fast/ruby/ruby-expansion-cjk-5.html platform/mac/svg/fonts/svg-font-general.html platform/mac/fast/text/rounding-hacks.html fast/ruby/ruby-expansion-cjk-2.html fast/ruby/ruby-expansion-cjk-4.html fast/writing-mode/vertical-font-vmtx-units-per-em.html fast/text/text-combine-different-fonts.html fast/writing-mode/text-orientation-basic.html
Created attachment 252503 [details] Archive of layout-test-results from ews102 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 252497 [details] Patch for landing Attachment 252497 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5326887270219776 New failing tests: fast/ruby/ruby-expansion-cjk-3.html fast/ruby/ruby-expansion-cjk.html http/tests/webfont/fallback-font-while-loading.html fast/ruby/ruby-expansion-cjk-5.html platform/mac/svg/fonts/svg-font-general.html platform/mac/fast/text/rounding-hacks.html fast/ruby/ruby-expansion-cjk-2.html fast/ruby/ruby-expansion-cjk-4.html fast/writing-mode/vertical-font-vmtx-units-per-em.html fast/text/text-combine-different-fonts.html fast/writing-mode/text-orientation-basic.html
Created attachment 252507 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 252497 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=252497&action=review > Source/WebCore/rendering/RenderText.cpp:549 > + maxW -= font.spaceWidth(); This caused this test to fail: fast/writing-mode/vertical-font-vmtx-units-per-em.html > Source/WebCore/rendering/RenderText.cpp:715 > + float wordTrailingSpaceWidth = font.typesettingFeatures() & Kerning ? font.spaceWidth() + wordSpacing : 0; This caused this test to fail: fast/writing-mode/text-orientation-basic.html > Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp:184 > + width = m_style.font.spaceWidth(); This caused this test to fail: platform/mac/fast/text/rounding-hacks.html > Source/WebCore/rendering/line/BreakingContext.h:743 > + float wordTrailingSpaceWidth = (font.typesettingFeatures() & Kerning) && !textLayout ? font.spaceWidth() + wordSpacing : 0; This caused these tests to fail: fast/ruby/ruby-expansion-cjk-2.html fast/ruby/ruby-expansion-cjk-3.html fast/ruby/ruby-expansion-cjk-4.html fast/ruby/ruby-expansion-cjk-5.html fast/ruby/ruby-expansion-cjk.html fast/text/text-combine-different-fonts.html http/tests/webfont/fallback-font-while-loading.html platform/mac/svg/fonts/svg-font-general.html
http/tests/webfont/fallback-font-while-loading.html shows that this patch is fundamentally incorrect. In particular, the implementation of FontCacade::spaceWidth() is this: float spaceWidth() const { return primaryFont().spaceWidth() + m_letterSpacing; } The fact that this disregards fallback fonts means that this function really shouldn't be used as-is. Instead, this functions should have the behavior of the replaced code in this patch.
(In reply to comment #9) > http/tests/webfont/fallback-font-while-loading.html shows that this patch is > fundamentally incorrect. In particular, the implementation of > FontCacade::spaceWidth() is this: > > float spaceWidth() const { return primaryFont().spaceWidth() + > m_letterSpacing; } > > The fact that this disregards fallback fonts means that this function really > shouldn't be used as-is. Instead, this functions should have the behavior of > the replaced code in this patch. ...Aaaaand we can't use this approach in FontCascade because SVG fonts require a RenderObject for context. This requirements goes away as soon as SVG fonts move entirely to the SVG -> OTF Font Converter.
Blocked by https://bugs.webkit.org/show_bug.cgi?id=143402