Bug 167219 - Simple line layout: Extend coverage for justified content.
Summary: Simple line layout: Extend coverage for justified content.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-19 16:46 PST by zalan
Modified: 2017-01-19 18:53 PST (History)
8 users (show)

See Also:


Attachments
Patch (7.99 KB, patch)
2017-01-19 17:00 PST, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2017-01-19 16:46:09 PST
with a range of punctuation characters.
Comment 1 zalan 2017-01-19 16:50:16 PST
rdar://problem/30108391
Comment 2 zalan 2017-01-19 17:00:39 PST
Created attachment 299286 [details]
Patch
Comment 3 zalan 2017-01-19 17:02:07 PST
Alternatively we could pass in a list of ranges, but i don't think the complexity is justified at this point (maybe when some other properties need to include/exclude certain unicode ranges)
Comment 4 Antti Koivisto 2017-01-19 17:16:20 PST
Comment on attachment 299286 [details]
Patch

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

> Source/WebCore/rendering/SimpleLineLayout.cpp:157
> +            // Include characters up to Latin Extended-B and some punctuation range when text is justified.
> +            bool isLatinIncludingExtendedB = character <= 0x01FF;
> +            bool isPunctuationRange = character >= 0x2010 && character <= 0x2027;
> +            if (!(isLatinIncludingExtendedB || isPunctuationRange))
> +                SET_REASON_AND_RETURN_IF_NEEDED(FlowHasJustifiedNonLatinText, reasons, includeReasons);
> +        }

You could also do this with ICU methods, something along the lines of

auto block = ublock_getCode(character);
bool ok = block <= UBLOCK_LATIN_EXTENDED_B || block == UBLOCK_GENERAL_PUNCTUATION
...

Not sure if it is better.

I suppose the range could be wider too.
Comment 5 zalan 2017-01-19 18:08:33 PST
(In reply to comment #4)
> Comment on attachment 299286 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299286&action=review
> 
> > Source/WebCore/rendering/SimpleLineLayout.cpp:157
> > +            // Include characters up to Latin Extended-B and some punctuation range when text is justified.
> > +            bool isLatinIncludingExtendedB = character <= 0x01FF;
> > +            bool isPunctuationRange = character >= 0x2010 && character <= 0x2027;
> > +            if (!(isLatinIncludingExtendedB || isPunctuationRange))
> > +                SET_REASON_AND_RETURN_IF_NEEDED(FlowHasJustifiedNonLatinText, reasons, includeReasons);
> > +        }
> 
> You could also do this with ICU methods, something along the lines of
> 
> auto block = ublock_getCode(character);
> bool ok = block <= UBLOCK_LATIN_EXTENDED_B || block ==
> UBLOCK_GENERAL_PUNCTUATION
> ...
> 
> Not sure if it is better.
> 
> I suppose the range could be wider too.
or narrower too. (0x2010, 0x2027) is a subset of the general punctuation range.
Comment 6 WebKit Commit Bot 2017-01-19 18:53:47 PST
Comment on attachment 299286 [details]
Patch

Clearing flags on attachment: 299286

Committed r210948: <http://trac.webkit.org/changeset/210948>
Comment 7 WebKit Commit Bot 2017-01-19 18:53:52 PST
All reviewed patches have been landed.  Closing bug.