[LayoutNG] Implement more text-align values and BiDi base direction

[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
Review-Url: https://codereview.chromium.org/2940153002
Cr-Commit-Position: refs/heads/master@{#481080}
Committed: https://chromium.googlesource.com/chromium/src/+/3cfd4f26430b0625455aa5abf90b2ed4ec129070

[kojii, 2017-06-15 19:34:42]

Description was changed from

==========
align

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

to

==========
align

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[kojii, 2017-06-16 14:09:16]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 14:09:32]

Dry run: CQ is trying da patch.

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

[kojii, 2017-06-16 14:24:40]

Description was changed from

==========
align

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Support more text-align values and BiDi base direction

This patch supports more 'text-align' values: 'left', 'center', 'start',
'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[kojii, 2017-06-16 14:27:12]

Description was changed from

==========
[LayoutNG] Support more text-align values and BiDi base direction

This patch supports more 'text-align' values: 'left', 'center', 'start',
'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Support more text-align values and BiDi base direction

This patch supports more 'text-align' values: 'left', 'center', 'start',
'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[kojii, 2017-06-16 14:27:38]

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

[kojii, 2017-06-16 14:27:38]

PTAL.

[kojii, 2017-06-16 14:36:40]

Description was changed from

==========
[LayoutNG] Support more text-align values and BiDi base direction

This patch supports more 'text-align' values: 'left', 'center', 'start',
'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', 'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[kojii, 2017-06-16 14:37:07]

Description was changed from

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', 'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[kojii, 2017-06-16 14:52:38]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 14:52:52]

Dry run: CQ is trying da patch.

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

[kojii, 2017-06-16 14:53:05]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-06-16 16:18:54]

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

[commit-bot: I haz the power, 2017-06-16 16:18:55]

Dry run: 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_...)

[kojii, 2017-06-19 05:43:22]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-19 05:43:29]

Dry run: 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, 2017-06-19 07:52:03]

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

[commit-bot: I haz the power, 2017-06-19 07:52:04]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[eae, 2017-06-19 15:36:49]

https://codereview.chromium.org/2940153002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
(right):

https://codereview.chromium.org/2940153002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:347:
: (available_width - inline_size) / 2;
Could you explain the math here please? If inline_size > available_width
(available_width - inline_size) would be negative, pushing the line outside of
the box. That doesn't seem right to me.

[kojii, 2017-06-20 03:52:20]

https://codereview.chromium.org/2940153002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
(right):

https://codereview.chromium.org/2940153002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:347:
: (available_width - inline_size) / 2;
On 2017/06/19 at 15:36:49, eae wrote:
> Could you explain the math here please? If inline_size > available_width
(available_width - inline_size) would be negative, pushing the line outside of
the box. That doesn't seem right to me.

It's saying: "if RTL and overflow, do 'right' align", and if RTL right align
overflows, it pushes the line outside to left, as in ETextAlign::kRight does.

But I agree this is very hard to read, I had to read this section of
LayoutBlockFlow::UpdateLogicalWidthForAlignment() multiple times to understand.
I'll try to make it more readable with comments.

[kojii, 2017-06-20 04:05:04]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 04:05:24]

Dry run: CQ is trying da patch.

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

[kojii, 2017-06-20 04:05:59]

done, PTAL. (I shouldn't have required reviewers to have the same pain as I had,
sorry m(__)m )

[commit-bot: I haz the power, 2017-06-20 06:07:13]

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

[commit-bot: I haz the power, 2017-06-20 06:07:14]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[kojii, 2017-06-20 17:01:10]

Description was changed from

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

'text-align: justify' is to be worked out.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

[eae, 2017-06-20 17:54:41]

LGTM

https://codereview.chromium.org/2940153002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/inline/ng_bidi_paragraph.h
(right):

https://codereview.chromium.org/2940153002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/inline/ng_bidi_paragraph.h:40: bool
IsAllLtr() const { return ubidi_getDirection(ubidi_) == UBIDI_LTR; }
Instead of IsAllLtr() could we make this more explicit and have
IsUndirectional() instead?

That way the calling code would read:
if (IsUndirectional() && BaseDirection() == kLtr) 

which is more explicit and less likely to cause call sites to incorrectly use
IsAllLtr() instead of BaseDirection() == kLtr or the other way around.

https://codereview.chromium.org/2940153002/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
(right):

https://codereview.chromium.org/2940153002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:319:
// TODO(kojii): Investigate handling trailing spaces.
You might want to clarify that this is only for white-space: pre|pre-wrap as in
all other cases they should have been stripped already.

https://codereview.chromium.org/2940153002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc:342:
*line_left +=
Much easier to follow now, thank you!

[kojii, 2017-06-20 18:59:02]

Patchset #6 (id:120001) has been deleted

[kojii, 2017-06-20 19:20:16]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[kojii, 2017-06-20 19:20:35]

Patchset #6 (id:140001) has been deleted

[commit-bot: I haz the power, 2017-06-20 19:21:00]

Dry run: 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, 2017-06-20 21:16:55]

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

[commit-bot: I haz the power, 2017-06-20 21:16:56]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[kojii, 2017-06-21 01:50:09]

Description was changed from

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
==========

to

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
==========

[kojii, 2017-06-21 01:50:45]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-06-21 01:50:46]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2940153002/#ps160001
(title: "eae review")

[commit-bot: I haz the power, 2017-06-21 01:51:08]

CQ is trying da patch.

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

[kojii, 2017-06-21 01:51:24]

All done, thank you!

[commit-bot: I haz the power, 2017-06-21 01:55:47]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1498009845530690,
"parent_rev": "7dabf71463f19ddf697ca33524183ba36b743f46", "commit_rev":
"3cfd4f26430b0625455aa5abf90b2ed4ec129070"}

[commit-bot: I haz the power, 2017-06-21 01:56:00]

Description was changed from

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993
==========

to

==========
[LayoutNG] Implement more text-align values and BiDi base direction

This patch implements more 'text-align' values: 'left', 'center',
'start', and 'end' (and its -webkit- versions.)

'text-align: justify' is to be worked out.

Also supports the base direction, a.k.a. paragraph direction, as defined
in UAX#9[1] for 'start' and 'end' to work. This support includes the
heuristic resolution by 'unicode-bidi: plaintext'[2].

The heuristics for forced line breaks is not implemented yet. The
failure of block-plaintext-004.html is caused by this.

[1] http://unicode.org/reports/tr9/
[2] https://drafts.csswg.org/css-writing-modes-3/#valdef-unicode-bidi-plaintext

BUG=636993

Review-Url: https://codereview.chromium.org/2940153002
Cr-Commit-Position: refs/heads/master@{#481080}
Committed:
https://chromium.googlesource.com/chromium/src/+/3cfd4f26430b0625455aa5abf90b...
==========

[commit-bot: I haz the power, 2017-06-21 01:56:02]

Committed patchset #6 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/3cfd4f26430b0625455aa5abf90b...