[webkit-reviews] review granted: [Bug 43467] Allow the language for hyphenation to be specified : [Attachment 63418] Add -webkit-hyphenate-locale

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 43467: Allow the language for hyphenation to be specified
https://bugs.webkit.org/show_bug.cgi?id=43467

Attachment 63418: Add -webkit-hyphenate-locale
https://bugs.webkit.org/attachment.cgi?id=63418&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +static CFLocaleRef getCFLocale(const AtomicString& localeIdentifier)

Can you come up with a name for this that doesn't include the word "get"? Maybe
you could just use the word "find" instead? Or "compute"?

> +static bool localeIsEnglish(const AtomicString& localeIdentifier)

Is there any way for this function to share code with getCFLocale in
HyphenationCF.cpp? They seem to have many many lines of code in common. Maybe a
template or something? I suppose HyphenationMac.mm will be deleted once
SnowLeopard is obsolete, but that might take a long time!

> -	       bool canHyphenate = style->hyphens() == HyphensAuto;
> +	       bool canHyphenate = style->hyphens() == HyphensAuto &&
WebCore::canHyphenate(style->hyphenateLocale());

"hyphenate locale" is such an unfortunate term. I know the CSS property has to
have that name because it's in the standard, but "hyphenation locale" is so
much more readable. Same thought about "hyphenate character", perhaps
"hyphenation string" is the best name to use internally in places like
RenderStyle.