Summary: | Fonts lie about being monospaced | ||
---|---|---|---|
Product: | WebKit | Reporter: | Hunseop Jeong <hs85.jeong> |
Component: | Text | Assignee: | Myles C. Maxfield <mmaxfield> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | buildbot, changseok, commit-queue, darin, dino, esprehn+autocc, ews-watchlist, frankhome61, glenn, hs85.jeong, jonlee, kondapallykalyan, mmaxfield, pdr, rniwa, simon.fraser, thorton, webkit-bug-importer, WebkitBugTracker, zalan |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Attachments: |
Created attachment 289805 [details]
Acme-Regular.ttf
Created attachment 289806 [details]
tick.ttf
Created attachment 289807 [details]
Actual.png
Created attachment 289808 [details]
Expected.png
mmaxifield, Do you know what is wrong? The font says that it is monospaced, but it is incorrect. The space character is not as wide as the rest of the characters. Created attachment 290573 [details]
Needs a test font
Comment on attachment 290573 [details] Needs a test font Attachment 290573 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2215828 New failing tests: fast/text/line-break-after-question-mark.html fast/regions/box-decorations-over-region-padding-fragmented.html Created attachment 290575 [details]
Archive of layout-test-results from ews102 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290573 [details] Needs a test font Attachment 290573 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2215840 New failing tests: fast/text/line-break-after-question-mark.html fast/regions/box-decorations-over-region-padding-fragmented.html Created attachment 290578 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 290573 [details] Needs a test font Attachment 290573 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2215850 New failing tests: fast/text/line-break-after-question-mark.html Created attachment 290581 [details]
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 290573 [details] Needs a test font Attachment 290573 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2215839 New failing tests: fast/text/line-break-after-question-mark.html fast/regions/box-decorations-over-region-padding-fragmented.html Created attachment 290582 [details]
Archive of layout-test-results from ews113 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 290753 [details]
Patch
Created attachment 290759 [details]
Patch
Comment on attachment 290759 [details]
Patch
Does this have performance implications?
Comment on attachment 290759 [details] Patch Attachment 290759 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2227744 New failing tests: fast/text/font-erroneous-monospace.html Created attachment 290764 [details]
Archive of layout-test-results from ews124 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 290759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290759&action=review Test is failing on iOS Simulator. Please don’t land without fixing that. > Source/WebCore/ChangeLog:16 > + When a font reports itself to be monospace, we use this as a > + signal that we can perform width computations by assuming all > + characters have the same width as the space character. However, > + some fonts erroneously claim to be monospaced. Firefox and > + Chrome both do not use this signal, so therefore they both > + correctly render these fonts. We should ignore this bit in the > + font as well. Also, CJK fonts generally do not have this bit > + set (because they usually have at least one character which is > + not fullwidth) so this isn't a concern there. I understand that you are saying that this optimization was implemented incorrectly. You seem to be saying either you determined that we don’t need the optimization, or that even though the optimization would be nice there is no practical way to do it correctly. Which one of those is the rationale? How did you determine that? > LayoutTests/fast/text/script-tests/line-break-after-question-mark.js:11 > - return div.offsetHeight > 25; > + return div.offsetHeight > 34; Is the best change to alter this number, or is the best test removing the character that triggers fallback? *** Bug 215727 has been marked as a duplicate of this bug. *** I wasn’t expecting that we’d wait 4 years after that review. PLT says this patch isn't a regression. $ compare-results -a plt-results-vanilla -b plt-results-patch a mean = 699.26720 b mean = 698.09886 pValue = 0.9183320159 (Smaller means are better.) 1.002 times better Results ARE NOT significant Comment on attachment 290759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290759&action=review >> LayoutTests/fast/text/script-tests/line-break-after-question-mark.js:11 >> + return div.offsetHeight > 34; > > Is the best change to alter this number, or is the best test removing the character that triggers fallback? We should remove the characters that triggers fallback. Ahem doesn't support U+0027 and U+007F. Created attachment 407181 [details]
Patch for committing
Committed r266118: <https://trac.webkit.org/changeset/266118> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407181 [details]. |
Created attachment 289804 [details] index.html Test: index.html with attached font. Expected: Expected.png (Chrome, firefox) Actual: Actual.png (Safari) Some characters is displayed over the right edge of the block if using the 'tick'.