Bug 69656

Summary:

[Skia on Chromium Mac] Set canExpandAroundIdeographsInComplexText to true

Product:

WebKit

Reporter:

Cary Clark <caryclark>

Component:

New Bugs

Assignee:

Cary Clark <caryclark>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

reed, senorblanco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Cary Clark 2011-10-07 12:33:20 PDT

[Skia on Chromium Mac] Set canExpandAroundIdeographsInComplexText to true

Comment 1 Cary Clark 2011-10-07 12:39:32 PDT

Created attachment 110197 [details]
Patch

Comment 2 Stephen White 2011-10-07 12:46:25 PDT

Comment on attachment 110197 [details]
Patch

Looks good (up to my limited knowledge).  Looping in Mike in case he has any comments.

As a side note, I see justify-ideograph-complex is failing on all (chrome) platforms currently.  Does it simply require rebaselining, or are there further problems?

Comment 3 Mike Reed 2011-10-07 12:58:44 PDT

Hmmm, seems fine, but confusing. The result of this function (which is only called once) seems to only mean "can I call isCJKIdeographOrSymbol() or not". But that guy is purely a unicode lookup function, with no platform specificisms afaik.

If it makes more pages look better, lets turn it on and see. From a code-review p.o.v. I can't see why this isn't on for all of our platforms (but time will tell).

Comment 4 Cary Clark 2011-10-07 13:17:40 PDT

justify-ideograph-complex should have a fully justified right side. Since canExpandAroundIdeographsInComplexText has never been enabled for Windows and Linux, this test has never succeeded on those platforms.

Oddly, there is a non-justified (i.e. wrong) expected image checked in for chromium-cg-mac.

Comment 5 Cary Clark 2011-10-07 13:19:37 PDT

Stephen, is there anything more you'd like me to do with this patch?

Comment 6 Stephen White 2011-10-07 13:22:33 PDT

Comment on attachment 110197 [details]
Patch

I'm going with Mike's OK.  r=me

Comment 7 Stephen White 2011-10-07 13:23:38 PDT

I've set CQ+ as well, but if you can't be around to watch the canaries when it lands, please clear it until you can.

Comment 8 Cary Clark 2011-10-07 13:27:52 PDT

Comment on attachment 110197 [details]
Patch

I'll wait 'til Monday morning to commit so I can watch the Canaries.

Comment 9 WebKit Review Bot 2011-10-10 06:07:00 PDT

Comment on attachment 110197 [details]
Patch

Clearing flags on attachment: 110197

Committed r97058: <http://trac.webkit.org/changeset/97058>

Comment 10 WebKit Review Bot 2011-10-10 06:07:05 PDT

All reviewed patches have been landed.  Closing bug.