Render Unicode control characters

Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org

Committed: https://crrev.com/275c35fe82bd295a75c0d555db0e0b26fcdf980b
Cr-Commit-Position: refs/heads/master@{#403542}

[eae, 2016-05-10 20:16:19]

Description was changed from

==========
space

BUG=
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html

BUG=530342
R=kojii@chromium.org
==========

[eae, 2016-05-10 20:16:19]

eae@chromium.org changed reviewers:
+ kojii@chromium.org

[eae, 2016-05-10 20:16:34]

Patchset #1 (id:1) has been deleted

[eae, 2016-05-10 20:19:14]

eae@chromium.org changed reviewers:
+ behdad@chromium.org

[eae, 2016-05-10 22:27:12]

Need to update the patch to still replace whitespace characters with space.

[kojii, 2016-05-10 23:41:22]

On 2016/05/10 at 22:27:12, eae wrote:
> Need to update the patch to still replace whitespace characters with space.

Yeah, so the "if (treatAsSpace()..." will remain?

We don't seem to have U+000D in treatAsSpace(), is this trimmed in parser or
somewhere else?

By comparing the list of treatAsZeroWidthSpaceInComplexScript() with Unicode
default ignorable:
http://unicode.org/cldr/utility/list-unicodeset.jsp?a=[%3ADI%3A]&g=
Unicode default ignorable is super-set except:
* U+0000-U+001F
* U+007F-U+00A0
* U+FFFC (objectReplacementCharacter)
so these in tests might be rendered.

If you prefer the spec link in the description:
https://drafts.csswg.org/css-text-3/#white-space-processing

[eae, 2016-05-11 16:25:28]

On 2016/05/10 23:41:22, kojii wrote:
> On 2016/05/10 at 22:27:12, eae wrote:
> > Need to update the patch to still replace whitespace characters with space.
> 
> Yeah, so the "if (treatAsSpace()..." will remain?
> 
> We don't seem to have U+000D in treatAsSpace(), is this trimmed in parser or
> somewhere else?
> 
> By comparing the list of treatAsZeroWidthSpaceInComplexScript() with Unicode
> default ignorable:
> http://unicode.org/cldr/utility/list-unicodeset.jsp?a=[%3ADI%3A]&g=
> Unicode default ignorable is super-set except:
> * U+0000-U+001F
> * U+007F-U+00A0
> * U+FFFC (objectReplacementCharacter)
> so these in tests might be rendered.
> 
> If you prefer the spec link in the description:
> https://drafts.csswg.org/css-text-3/#white-space-processing

Thanks, I plan to add a defaultIgnorable helper and base it on the spec.

[kojii, 2016-05-11 17:56:56]

On 2016/05/11 at 16:25:28, eae wrote:
> 
> Thanks, I plan to add a defaultIgnorable helper and base it on the spec.

You might already found but ICU seems to have a property
UPROPS_DEFAULT_IGNORABLE_CODE_POINT
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/icu/so...
I hope this serves.

[eae, 2016-05-16 21:35:41]

Description was changed from

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html

BUG=530342
R=kojii@chromium.org
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

WWW: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

BUG=530342
R=kojii@chromium.org
==========

[eae, 2016-05-16 21:35:52]

Description was changed from

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

WWW: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

BUG=530342
R=kojii@chromium.org
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

BUG=530342
R=kojii@chromium.org
==========

[eae, 2016-05-17 20:00:22]

PTAL

[kojii, 2016-05-18 00:07:16]

I think I confused more than necessary, sorry about that.

How about move 0xA0 and objectReplacementCharacter back?

0xA0 isn't Cc so that can be still ignored. objectReplacementCharacter doesn't
care much so either way doesn't look to matter much.

[behdad, 2016-05-19 01:37:25]

lgtm

[kojii, 2016-05-19 03:44:59]

https://codereview.chromium.org/1964773002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/Character.h (right):

https://codereview.chromium.org/1964773002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/Character.h:108: || c ==
carriageReturnCharacter
|| c == noBreakSpaceCharacter
? 0xA0 should not be legacy.

[eae, 2016-06-28 16:55:28]

Patchset #1 (id:20001) has been deleted

[eae, 2016-06-28 16:55:50]

Patchset #1 (id:40001) has been deleted

[eae, 2016-06-28 16:55:59]

Patchset #1 (id:60001) has been deleted

[eae, 2016-06-28 16:56:08]

Patchset #1 (id:80001) has been deleted

[eae, 2016-06-28 17:56:50]

PTAL

[kojii, 2016-06-28 18:58:15]

Logic lgtm, except missing newlineCharacter below unless you omitted it
intentionally, but test failures looks we need a bit more work.

fast/repaint/continuation-after-outline.html maybe the lack of newlineCharacter
but not sure why only this fails, maybe the cause is different and we don't need
newlineCharacter?

Crashes are probably HashTable<UChar32> needs HashTraits to store U+0000.

fast/text/zero-width-characters-complex-script.html might need to tweak the test
(or rebase?).

The rests are tiny but a bit time consuming, please let me know if I should take
over. This work doesn't look to worth your time ;)

https://codereview.chromium.org/1964773002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/Character.h (right):

https://codereview.chromium.org/1964773002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/Character.h:108: || c ==
carriageReturnCharacter
newlineCharacter not needed?

[eae, 2016-06-28 20:40:15]

On 2016/06/28 18:58:15, kojii wrote:
> Logic lgtm, except missing newlineCharacter below unless you omitted it
> intentionally, but test failures looks we need a bit more work.
> 
> fast/repaint/continuation-after-outline.html maybe the lack of
newlineCharacter
> but not sure why only this fails, maybe the cause is different and we don't
need
> newlineCharacter?
> 
> Crashes are probably HashTable<UChar32> needs HashTraits to store U+0000.
> 
> fast/text/zero-width-characters-complex-script.html might need to tweak the
test
> (or rebase?).
> 
> The rests are tiny but a bit time consuming, please let me know if I should
take
> over. This work doesn't look to worth your time ;)
> 
>
https://codereview.chromium.org/1964773002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/platform/text/Character.h (right):
> 
>
https://codereview.chromium.org/1964773002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/platform/text/Character.h:108: || c ==
> carriageReturnCharacter
> newlineCharacter not needed?

Newline is handled in treatAsSpace which is called before.

[eae, 2016-06-28 20:40:58]

On 2016/06/28 20:40:15, eae wrote:
> On 2016/06/28 18:58:15, kojii wrote:
> > Logic lgtm, except missing newlineCharacter below unless you omitted it
> > intentionally, but test failures looks we need a bit more work.
> > 
> > fast/repaint/continuation-after-outline.html maybe the lack of
> newlineCharacter
> > but not sure why only this fails, maybe the cause is different and we don't
> need
> > newlineCharacter?
> > 
> > Crashes are probably HashTable<UChar32> needs HashTraits to store U+0000.
> > 
> > fast/text/zero-width-characters-complex-script.html might need to tweak the
> test
> > (or rebase?).
> > 
> > The rests are tiny but a bit time consuming, please let me know if I should
> take
> > over. This work doesn't look to worth your time ;)

I don't mind doing it as long as we get the characters right. Sometimes it's
nice to have a mindless task to do in between other work.

[eae, 2016-06-28 21:10:13]

Description was changed from

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

BUG=530342
R=kojii@chromium.org
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org
==========

[eae, 2016-06-29 16:23:20]

The CQ bit was checked by eae@chromium.org

[eae, 2016-06-29 16:23:20]

The patchset sent to the CQ was uploaded after l-g-t-m from behdad@chromium.org,
kojii@chromium.org
Link to the patchset: https://codereview.chromium.org/1964773002/#ps140001
(title: "w/Test changes")

[commit-bot: I haz the power, 2016-06-29 16:24:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 16:35:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-29 16:35:40]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pdr., 2016-06-30 18:32:44]

On 2016/06/29 at 16:35:40, commit-bot wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

LGTM

[eae, 2016-06-30 18:33:22]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-06-30 18:33:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 18:42:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 18:43:01]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[eae, 2016-06-30 19:18:57]

eae@chromium.org changed reviewers:
+ esprehn@chromium.org

[eae, 2016-06-30 19:18:58]

+esprehn for Source/wtf

[esprehn, 2016-06-30 19:45:34]

esprehn@chromium.org changed reviewers:
+ pdr@chromium.org

[esprehn, 2016-06-30 19:45:35]

lgtm

[eae, 2016-06-30 20:01:09]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-06-30 20:01:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-30 20:49:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 20:49:59]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eae, 2016-07-01 19:04:40]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2016-07-01 19:05:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 22:00:30]

Description was changed from

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org
==========

[commit-bot: I haz the power, 2016-07-01 22:00:32]

Committed patchset #3 (id:140001)

[commit-bot: I haz the power, 2016-07-01 22:00:45]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 22:02:28]

Description was changed from

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org
==========

to

==========
Render Unicode control characters

Change the shaping logic to no longer replace Unicode control characters
with space or zero-width-space to better match the spec. Implement as an
experimental web platform feature to ensure that it doesn't break sites.

W3C: https://lists.w3.org/Archives/Public/www-style/2014Mar/0475.html
Spec: https://drafts.csswg.org/css-text-3/#white-space-processing

TEST=fast/text/zero-width-characters-complex-script.html
BUG=530342
R=kojii@chromium.org

Committed: https://crrev.com/275c35fe82bd295a75c0d555db0e0b26fcdf980b
Cr-Commit-Position: refs/heads/master@{#403542}
==========

[commit-bot: I haz the power, 2016-07-01 22:02:30]

Patchset 3 (id:??) landed as
https://crrev.com/275c35fe82bd295a75c0d555db0e0b26fcdf980b
Cr-Commit-Position: refs/heads/master@{#403542}