Bug 53207 - [Chromium] Simplify small caps logic in complex text on linux
Summary: [Chromium] Simplify small caps logic in complex text on linux
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: James Simonsen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-26 17:18 PST by James Simonsen
Modified: 2011-01-27 15:03 PST (History)
2 users (show)

See Also:

Attachments
Patch (3.07 KB, patch)
2011-01-26 17:20 PST, James Simonsen 		no flags Details | Formatted Diff | Diff
Patch (3.17 KB, patch)
2011-01-27 12:58 PST, James Simonsen 		tony: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Simonsen 2011-01-26 17:18:04 PST 
[Chromium] Simplify small caps logic in complex text on linux
Comment 1 James Simonsen 2011-01-26 17:20:31 PST 
Created attachment 80274 [details]
Patch
Comment 2 James Simonsen 2011-01-27 10:56:40 PST 
This should simplify things a bit.
Comment 3 Tony Chang 2011-01-27 11:05:41 PST 
Comment on attachment 80274 [details]
Patch

The code looks good, but can you still add a test case where an uppercase run becomes lowercase (if one doesn't exist)?  That way we won't regress this in the future.
Comment 4 James Simonsen 2011-01-27 11:31:03 PST 
(In reply to comment #3)
> (From update of attachment 80274 [details])
> The code looks good, but can you still add a test case where an uppercase run becomes lowercase (if one doesn't exist)?  That way we won't regress this in the future.

I think the existing one already covers that with "Lorem ipsum." I'm not sure why that didn't occur to me earlier. :/

We also cover the opposite condition with "A?b?e" in atsui-small-caps-punctuation-size.html. It's not entirely intuitive, but punctuation is considered to be upper case.
Comment 5 Tony Chang 2011-01-27 12:02:01 PST 
Comment on attachment 80274 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80274&action=review

> Source/WebCore/ChangeLog:9
> +        No new tests.
> +

Nit: Please mention which tests cover this.
Comment 6 James Simonsen 2011-01-27 12:58:19 PST 
Created attachment 80358 [details]
Patch
Comment 7 James Simonsen 2011-01-27 12:58:44 PST 
(In reply to comment #5)
> (From update of attachment 80274 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80274&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        No new tests.
> > +
> 
> Nit: Please mention which tests cover this.

Done.
Comment 8 James Simonsen 2011-01-27 15:03:21 PST 
Committed r76845: <http://trac.webkit.org/changeset/76845>