Bug 162546 – Fonts lie about being monospaced

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Hunseop Jeong]

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'.

[Hunseop Jeong]

Created attachment 289805 [details]
Acme-Regular.ttf

[Hunseop Jeong]

Created attachment 289806 [details]
tick.ttf

[Hunseop Jeong]

Created attachment 289807 [details]
Actual.png

[Hunseop Jeong]

Created attachment 289808 [details]
Expected.png

[Hunseop Jeong]

mmaxifield, Do you know what is wrong?

[Radar WebKit Bug Importer]

<rdar://problem/28494654>

[Myles C. Maxfield]

The font says that it is monospaced, but it is incorrect. The space character is not as wide as the rest of the characters.

[Myles C. Maxfield]

Created attachment 290573 [details]
Needs a test font

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Myles C. Maxfield]

Created attachment 290753 [details]
Patch

[Myles C. Maxfield]

Created attachment 290759 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 290759 [details]
Patch

Does this have performance implications?

[Build Bot]

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

[Build Bot]

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

[Darin Adler]

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?

[Myles C. Maxfield]

*** Bug 215727 has been marked as a duplicate of this bug. ***

[Darin Adler]

I wasn’t expecting that we’d wait 4 years after that review.

[Myles C. Maxfield]

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

[Myles C. Maxfield]

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.

[Myles C. Maxfield]

Created attachment 407181 [details]
Patch for committing

[EWS]

Committed r266118: <https://trac.webkit.org/changeset/266118>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407181 [details].