Bug 123814 - Full width semicolon is wrong in vertical text
Summary: Full width semicolon is wrong in vertical text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-11-05 11:30 PST by Enrica Casucci
Modified: 2013-11-05 15:13 PST (History)
2 users (show)

See Also:


Attachments
Patch (15.54 KB, patch)
2013-11-05 13:21 PST, Enrica Casucci
benjamin: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (665.79 KB, application/zip)
2013-11-05 13:49 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (836.90 KB, application/zip)
2013-11-05 14:22 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2013-11-05 11:30:28 PST
The unicode character 0xFF1B should always be upright in vertical text.

<rdar://problem/15312541>
Comment 1 Enrica Casucci 2013-11-05 13:21:30 PST
Created attachment 216065 [details]
Patch
Comment 2 Benjamin Poulain 2013-11-05 13:33:40 PST
Comment on attachment 216065 [details]
Patch

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

> Source/WebCore/platform/graphics/FontGlyphs.cpp:194
>      if (isInRange(character, 0x0FF01, 0x0FF07) || isInRange(character, 0x0FF0A, 0x0FF0C)
> -        || isInRange(character, 0x0FF0E, 0x0FF19) || isInRange(character, 0x0FF1F, 0x0FF3A))
> +        || isInRange(character, 0x0FF0E, 0x0FF19) || character == 0x0FF1B || isInRange(character, 0x0FF1F, 0x0FF3A))
>          return true;

I think you should name 0x0FF1B.

Something like:
    UChar32 fullWidthSemicolon = 0x0FF1B;
    if (... character == fullWidthSemicolon ...
Comment 3 Enrica Casucci 2013-11-05 13:48:14 PST
(In reply to comment #2)
> (From update of attachment 216065 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=216065&action=review
> 
> > Source/WebCore/platform/graphics/FontGlyphs.cpp:194
> >      if (isInRange(character, 0x0FF01, 0x0FF07) || isInRange(character, 0x0FF0A, 0x0FF0C)
> > -        || isInRange(character, 0x0FF0E, 0x0FF19) || isInRange(character, 0x0FF1F, 0x0FF3A))
> > +        || isInRange(character, 0x0FF0E, 0x0FF19) || character == 0x0FF1B || isInRange(character, 0x0FF1F, 0x0FF3A))
> >          return true;
> 
> I think you should name 0x0FF1B.
> 
> Something like:
>     UChar32 fullWidthSemicolon = 0x0FF1B;
>     if (... character == fullWidthSemicolon ...

This function contains already several other individual characters and ranges according to one of the sub specs of unicode and they are all unnamed, so I'll leave this one unnamed too.
Comment 4 Build Bot 2013-11-05 13:49:09 PST
Comment on attachment 216065 [details]
Patch

Attachment 216065 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21158087

New failing tests:
platform/mac/fast/text/vertical-no-sideways.html
Comment 5 Build Bot 2013-11-05 13:49:10 PST
Created attachment 216074 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2013-11-05 14:22:03 PST
Comment on attachment 216065 [details]
Patch

Attachment 216065 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/21278016

New failing tests:
platform/mac/fast/text/vertical-no-sideways.html
Comment 7 Build Bot 2013-11-05 14:22:05 PST
Created attachment 216080 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Enrica Casucci 2013-11-05 15:13:30 PST
Added new test results for mountain lion.
Committed revision 158697.