Commit d6415161 authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Commit Bot

Make TextOffsetMapping::InlineContents not to use LayoutRubyAsBlock as container

This patch changes |TextOffsetMapping::InlineContents| not to use
|LayoutRubyAsBlock| as container to avoid invalid DOM range.

In <ruby>abc<rt>XYZ</rt></ruby>, before this patch |InlineContents| use
<ruby> as container and having range start with <rt>@0 and "abc"[3] because
in layout tree, <rt> comes before "abc".

After this patch, we make |LayoutNGRubyRun| and |LayoutNGRubyBase| as
container but |LayoutNGRubyAsBlock|.

Layout tree:
      LayoutNGRubyAsBlock {RUBY} at (0,0) size 784x27
        LayoutNGRubyRun (anonymous) at (0,7) size 22x20
          LayoutNGRubyText {RT} at (0,-10) size 22x12
            LayoutText {#text} at (2,0) size 18x12
              text run at (2,0) width 18: "XYZ"
          LayoutNGRubyBase (anonymous) at (0,0) size 22x20
            LayoutText {#text} at (0,0) size 22x19
              text run at (0,0) width 22: "abc"

Bug: 1124584
Change-Id: Id4006630a87127a6af2d1a5044a07e253457f8bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460551
Commit-Queue: Kent Tamura <tkent@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815433}
parent 0c9a8f77
...@@ -40,6 +40,12 @@ bool CanBeInlineContentsContainer(const LayoutObject& layout_object) { ...@@ -40,6 +40,12 @@ bool CanBeInlineContentsContainer(const LayoutObject& layout_object) {
return false; return false;
if (!block_flow->ChildrenInline() || block_flow->IsAtomicInlineLevel()) if (!block_flow->ChildrenInline() || block_flow->IsAtomicInlineLevel())
return false; return false;
if (block_flow->IsRuby()) {
// We should not make |LayoutRubyAsBlock| as inline contents container,
// because ruby base text comes after ruby text in layout tree.
// See ParameterizedTextOffsetMappingTest.RangeOfBlockWithRubyAsBlock
return false;
}
if (block_flow->NonPseudoNode()) { if (block_flow->NonPseudoNode()) {
// It is OK as long as |block_flow| is associated to non-pseudo |Node| even // It is OK as long as |block_flow| is associated to non-pseudo |Node| even
// if it is empty block or containing only anonymous objects. // if it is empty block or containing only anonymous objects.
......
...@@ -196,6 +196,39 @@ TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRUBY) { ...@@ -196,6 +196,39 @@ TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRUBY) {
GetRange("<ruby>abc<rt>1|23</rt></ruby>")); GetRange("<ruby>abc<rt>1|23</rt></ruby>"));
} }
// http://crbug.com/1124584
TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRubyAsBlock) {
// We should not make <ruby> as |InlineContent| container because "XYZ" comes
// before "abc" but in DOM tree, order is "abc" then "XYZ".
// Layout tree:
// LayoutNGBlockFlow {BODY} at (8,8) size 784x27
// LayoutNGRubyAsBlock {RUBY} at (0,0) size 784x27
// LayoutNGRubyRun (anonymous) at (0,7) size 22x20
// LayoutNGRubyText {RT} at (0,-10) size 22x12
// LayoutText {#text} at (2,0) size 18x12
// text run at (2,0) width 18: "XYZ"
// LayoutNGRubyBase (anonymous) at (0,0) size 22x20
// LayoutText {#text} at (0,0) size 22x19
// text run at (0,0) width 22: "abc"
InsertStyleElement("ruby { display: block; }");
EXPECT_EQ("<ruby>^abc|<rt>XYZ</rt></ruby>",
GetRange("|<ruby>abc<rt>XYZ</rt></ruby>"));
EXPECT_EQ("<ruby>^abc|<rt>XYZ</rt></ruby>",
GetRange("<ruby>|abc<rt>XYZ</rt></ruby>"));
EXPECT_EQ("<ruby>abc<rt>^XYZ|</rt></ruby>",
GetRange("<ruby>abc<rt>|XYZ</rt></ruby>"));
}
TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRubyAsInlineBlock) {
InsertStyleElement("ruby { display: inline-block; }");
EXPECT_EQ("<ruby>^abc|<rt>XYZ</rt></ruby>",
GetRange("|<ruby>abc<rt>XYZ</rt></ruby>"));
EXPECT_EQ("<ruby>^abc|<rt>XYZ</rt></ruby>",
GetRange("<ruby>|abc<rt>XYZ</rt></ruby>"));
EXPECT_EQ("<ruby>abc<rt>^XYZ|</rt></ruby>",
GetRange("<ruby>abc<rt>|XYZ</rt></ruby>"));
}
TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRUBYandBR) { TEST_P(ParameterizedTextOffsetMappingTest, RangeOfBlockWithRUBYandBR) {
EXPECT_EQ("<ruby>^abc<br>def|<rt>123<br>456</rt></ruby>", EXPECT_EQ("<ruby>^abc<br>def|<rt>123<br>456</rt></ruby>",
GetRange("<ruby>|abc<br>def<rt>123<br>456</rt></ruby>")) GetRange("<ruby>|abc<br>def<rt>123<br>456</rt></ruby>"))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment