Make -webkit-font-kerning actually work.

Make -webkit-font-kerning actually work.

Right now, the complex text path always kerns, even with
-webkit-font-kerning:none.

This is technically tested by fast/text/font-kerning.html, but the default font
often only has TrueType kerning, which isn't supported yet (bug 41990).
I've changed the test to prefer some fonts that are widely available and
actually get kerned.

BUG=165643
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162485

[efidler1, 2013-11-15 21:47:20]

ptal

[jamesr, 2013-11-15 21:48:57]

why do we have a vendor-prefixed CSS hook for this?  Is there anything standard
to control kerning?

[efidler1, 2013-11-15 21:50:46]

font-kerning is standard
(http://dev.w3.org/csswg/css-fonts/#propdef-font-kerning), we've just never
unprefixed it.

[jamesr, 2013-11-15 21:52:48]

If the vendor prefixed version is a no-op maybe we should just remove it and add
an unprefixed version that works.

[behdad_google, 2013-11-15 22:16:37]

https://codereview.chromium.org/74413002/diff/1/Source/core/platform/graphics...
File Source/core/platform/graphics/harfbuzz/HarfBuzzShaper.cpp (right):

https://codereview.chromium.org/74413002/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/harfbuzz/HarfBuzzShaper.cpp:517:
m_features.append(kern);
No need to explicitly turn on.  Default is turned on.  Adding it pollutes /
disables hb's internal cache.

[efidler1, 2013-11-15 22:17:17]

Yes, apologies. You already told me that. I'll fix.

[behdad_google, 2013-11-15 22:17:40]

On 2013/11/15 22:17:17, efidler1 wrote:
> Yes, apologies. You already told me that. I'll fix.

Np. Thanks.

[efidler1, 2013-11-15 23:27:22]

Ok, new version addresses Behdad's comments.

I'd like to fix this, and then address unprefixing afterwards, but James seems
to think that might be a waste of an opportunity. -webkit-font-kerning is
probably not currently a total no-op, because SimpleFontDataCoreText does seem
to address kerning, but I'm not totally sure how the Mac code works.

[behdad_google, 2013-11-15 23:36:34]

lgtm

[efidler1, 2013-11-18 13:33:38]

On 2013/11/15 23:36:34, behdad_google wrote:
> lgtm

thanks. jamesr, are you ok with landing this?

[jamesr, 2013-11-19 21:02:36]

I don't think it's worthwhile to improve a vendor-prefixed property when we
could instead ship a non-prefixed version that works.

[efidler1, 2013-11-20 22:15:05]

FWIW, -webkit-font-kerning does work (to some extent) on Chrome for Mac. It's a
total no-op on HarfBuzz (i.e. Linux).

[eseidel, 2013-11-20 23:51:03]

I spoke with Eli in person just now.  Although I 100% agree with James that we
*should not* be fixing -webkit- prefixes, that instead we should be removing
them, I think in this case we can make an exception.

The exception in this case, is that Eli will shortly be removing this property
(unprefixing it) but that that will be blocked on an Intent-to-ship.  I don't
think it hurts to leave this prefixed while that works through the system and we
can land this change independently of the unprefixing option.

-webkit-font-kerning only has 0.005% usage, which is only 50% higher than our
previous most-popular removal:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/bs3NIiqnc8Q/l9hyhrZL...
Which makes it an easy argument to just remove (currently non-working except for
Mac) prefixed version:
http://www.chromestatus.com/metrics/css/timeline/popularity/13

So I guess in short, lgtm.  James, you OK with this resolution?

[jamesr, 2013-11-20 23:53:24]

That sounds fine.  Having numbers makes it easier to make this call.

I think we should publicize a general policy on fixing vendor-prefixed
properties to blink-dev.  If the property was completely broken in all cases I
think that's a clear case for don't fix but the fact that it works in some of
our configs makes this case trickier.

[efidler1, 2013-11-21 18:04:06]

Ok, thanks all. Can someone land this for me?

[commit-bot: I haz the power, 2013-11-21 18:10:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/efidler@blackberry.com/74413002/170001

[commit-bot: I haz the power, 2013-11-21 19:04:47]

Change committed as 162485