Created attachment 319586 [details] letter-spacing in AudioWide Font between f & l letter for Chrome, Safari and Firefox Chrome Version : 60.0.3112.101 OS Version: OS X 10.12.5 URLs (if applicable) : Other browsers tested: Add OK or FAIL after other browsers where you have tested this issue: Safari 5: FAIL Firefox 4.x: OK IE 7/8/9: OK What steps will reproduce the problem? 1. Use audiowide font on the web 2. Ensure you have text like flash in h1 tag 3. Include letter-spacing for h1 tag. What is the expected result? letter-spacing should have created a gap between letters f & l. What happens instead of that? There is no letter-spacing between letters f & l. Please provide any additional information below. Attach a screenshot if possible. Attached screenshots for chrome, firefox UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.101 Safari/537.36
This is because we erroneously leave ligatures on, even when letter-spacing is enabled.
<rdar://problem/17044265>
Created attachment 319643 [details] Patch
Comment on attachment 319643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319643&action=review > Source/WebCore/ChangeLog:3 > + No letter spacing between f & l for audiowide font (not happening in firefox) Can this adopt the title from the bug, which is much better? > Source/WebCore/platform/graphics/FontDescription.h:142 > + setVariantDiscretionaryLigatures(FontVariantLigatures::No); > + setVariantHistoricalLigatures(FontVariantLigatures::No); Technically no test for these two?
Comment on attachment 319643 [details] Patch Attachment 319643 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4428781 New failing tests: fast/css/word-space-extra.html editing/mac/attributed-string/letter-spacing.html
Created attachment 319648 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 319643 [details] Patch Attachment 319643 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4428787 New failing tests: fast/css/word-space-extra.html
Created attachment 319650 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 319643 [details] Patch Attachment 319643 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4428809 New failing tests: fast/css/word-space-extra.html
Created attachment 319652 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
I have got rid of the issue by using css style font-variant-ligatures:none; I will leave this to the individual browser teams to decide if they should enable ligatures or not. I do not see any advantage of enabling ligatures by default unless the font is using cursive writing style. https://bugs.chromium.org/p/chromium/issues/detail?id=761281&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&groupby=&sort=
Created attachment 320588 [details] Patch for committing
Comment on attachment 320588 [details] Patch for committing Attachment 320588 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4529159 New failing tests: editing/mac/attributed-string/letter-spacing.html
Created attachment 320596 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Thinking out loud here, but is there a case where ligatures may still be desirable with negative letter-spacing? I guess at extreme values it wouldn't.
I don't think this patch is right, because other browsers don't let letter-spacing change the web-exposed computed value of font-variant-ligatures.
The CSS spec defines exactly how this should work: https://drafts.csswg.org/css-fonts-4/#feature-variation-precedence 11. Feature settings determined by properties other than font-variant or font-feature-settings are applied. For example, setting a non-default value for the letter-spacing property disables optional ligatures.
justification should disable ligatures, too.
"When the effective spacing between two characters is not zero (due to either justification or a non-zero value of letter-spacing), user agents should not apply optional ligatures" https://drafts.csswg.org/css-text-3/#letter-spacing-property
(In reply to Myles C. Maxfield from comment #18) > justification should disable ligatures, too. But only if justification adds space in between characters. text-justify:inter-character can do that[1], but WebKit doesn't implement that (yet). https://drafts.csswg.org/css-text-3/#text-justify-property
Created attachment 407949 [details] Patch
Created attachment 407950 [details] Patch
Created attachment 407951 [details] Patch
Comment on attachment 407951 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407951&action=review > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:536 > +// FIXME: This function is 180 lines long. We should factor it into smaller pieces. This seems bit unnecessary. It will also easily become a lie when someone adds more lines and forgets to update the number.
Created attachment 408068 [details] Patch for committing
Tools/Scripts/svn-apply failed to apply attachment 408068 [details] to trunk. Please resolve the conflicts and upload a new patch.
Created attachment 408088 [details] Patch for committing
Committed r266683: <https://trac.webkit.org/changeset/266683>