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

Make LayoutText::GetTextBoxInfo() not to crash ellipsis for content of ::after

This patch makes |LayoutText::GetTextBoxInfo()| explicitly ignores generated
text, e.g. ellipsis, instead of using fragment tree structure with
|IsHiddenForPaint()|.

Before this patch, text fragment for ellipsis is skipped by using tree
structure:
  1. fragment for original text marked hidden for paint
  2. fragment for trunated text
  3. fragment for ellipsis
When |GetTextBoxInfo()| vistis fragment#2, it stops processing then it doesn't
process a fragment for ellipsis.

However, for ellipsis for one character ::after, tree is
  1. fragment for ::after
  2. fragment for ellipsis
Then |GetTextBoxInfo()| handles ellipsis fragment and calls offset mapping with
text offset 0 and 1 of ellipsis text in text fragment instead of text content
of |NGInlineNodeData|. This causes |GetTextBoxInfo()| processes invalid mapping
result then crashes.

Bug: 1003413
Change-Id: I210738f609e78854d1d1887e1a5afa93aea40b64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808628
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697139}
parent af166a10
...@@ -297,6 +297,9 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const { ...@@ -297,6 +297,9 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const {
for (const NGPaintFragment* fragment : fragments) { for (const NGPaintFragment* fragment : fragments) {
const auto& text_fragment = const auto& text_fragment =
To<NGPhysicalTextFragment>(fragment->PhysicalFragment()); To<NGPhysicalTextFragment>(fragment->PhysicalFragment());
// TODO(yosin): We should introduce
// |NGPhysicalTextFragment::IsTruncated()| to skip them instead of using
// |IsHiddenForPaint()| with ordering of fragments.
if (text_fragment.IsHiddenForPaint()) { if (text_fragment.IsHiddenForPaint()) {
in_hidden_for_paint = true; in_hidden_for_paint = true;
} else if (in_hidden_for_paint) { } else if (in_hidden_for_paint) {
...@@ -304,6 +307,10 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const { ...@@ -304,6 +307,10 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const {
// ignore truncated fragments (actually painted). // ignore truncated fragments (actually painted).
break; break;
} }
// We don't put generated texts, e.g. ellipsis, hyphen, etc. not in text
// content, into results. Note: CSS "content" aren't categorized this.
if (text_fragment.TextType() == NGPhysicalTextFragment::kGeneratedText)
continue;
// When the corresponding DOM range contains collapsed whitespaces, NG // When the corresponding DOM range contains collapsed whitespaces, NG
// produces one fragment but legacy produces multiple text boxes broken at // produces one fragment but legacy produces multiple text boxes broken at
// collapsed whitespaces. We break the fragment at collapsed whitespaces // collapsed whitespaces. We break the fragment at collapsed whitespaces
...@@ -311,6 +318,7 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const { ...@@ -311,6 +318,7 @@ Vector<LayoutText::TextBoxInfo> LayoutText::GetTextBoxInfo() const {
for (const NGOffsetMappingUnit& unit : for (const NGOffsetMappingUnit& unit :
mapping->GetMappingUnitsForTextContentOffsetRange( mapping->GetMappingUnitsForTextContentOffsetRange(
text_fragment.StartOffset(), text_fragment.EndOffset())) { text_fragment.StartOffset(), text_fragment.EndOffset())) {
DCHECK_EQ(unit.GetLayoutObject(), this);
if (unit.GetType() == NGOffsetMappingUnitType::kCollapsed) if (unit.GetType() == NGOffsetMappingUnitType::kCollapsed)
continue; continue;
// [clamped_start, clamped_end] of |fragment| matches a legacy text box. // [clamped_start, clamped_end] of |fragment| matches a legacy text box.
......
...@@ -492,6 +492,37 @@ TEST_P(ParameterizedLayoutTextTest, GetTextBoxInfoWithEllipsis) { ...@@ -492,6 +492,37 @@ TEST_P(ParameterizedLayoutTextTest, GetTextBoxInfoWithEllipsis) {
EXPECT_EQ(LayoutRect(60, 0, 50, 10), boxes[1].local_rect); EXPECT_EQ(LayoutRect(60, 0, 50, 10), boxes[1].local_rect);
} }
// For http://crbug.com/1003413
TEST_P(ParameterizedLayoutTextTest, GetTextBoxInfoWithEllipsisForPseudoAfter) {
LoadAhem();
SetBodyInnerHTML(R"HTML(
<style>
#sample {
box-sizing: border-box;
font: 10px/1 Ahem;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
width: 5ch;
}
b::after { content: ","; }
</style>
<div id=sample><b id=target>abc</b><b>xyz</b></div>
)HTML");
const Element& target = *GetElementById("target");
const Element& after = *target.GetPseudoElement(kPseudoIdAfter);
// Set |layout_text| to "," in <pseudo::after>,</pseudo::after>
const LayoutText& layout_text =
*ToLayoutText(after.GetLayoutObject()->SlowFirstChild());
auto boxes = layout_text.GetTextBoxInfo();
EXPECT_EQ(1u, boxes.size());
EXPECT_EQ(0u, boxes[0].dom_start_offset);
EXPECT_EQ(1u, boxes[0].dom_length);
EXPECT_EQ(LayoutRect(30, 0, 10, 10), boxes[0].local_rect);
}
TEST_P(ParameterizedLayoutTextTest, TEST_P(ParameterizedLayoutTextTest,
IsBeforeAfterNonCollapsedCharacterNoLineWrap) { IsBeforeAfterNonCollapsedCharacterNoLineWrap) {
// Basic tests // Basic tests
......
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