Skip to content

GitLab

T
tangled
Commit abf98057 authored by Ryan Landay's avatar Ryan Landay Committed by Commit Bot
Browse files
Options

Fix off-by-one bug with Find In Page highlights caused by :first-letter 

This CL fixes a bug where using Find In Page to search for text in an element
using a :first-letter CSS psuedo element highlights one extra character at the
end of the result text and skips a character at the beginning.

We have the same bug in the function we use for other marker types (e.g.
composition and spelling) so I'm fixing it there too.

There's still another issue that this CL does not fix that prevents the first
letter itself from becoming highlighted. I am not yet sure what else needs to be
changed to fix this.

Bug: 798004
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: If1f09dd5485be8f2817e06b3b3ef65f8b1bb11ec
Reviewed-on: https://chromium-review.googlesource.com/847834Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526740}
parent 84077a2b
Hide whitespace changes
Inline Side-by-side
Showing with 111 additions and 7 deletions
third_party/WebKit/LayoutTests/paint/markers/first-letter.html 0 → 100644
View file @ abf98057
<!DOCTYPE html>
<script src="../../resources/run-after-layout-and-paint.js"></script>
<style>
div:first-letter {
color: red;
}
</style>
<div id="div">None Composition Spelling TextMatch</div>
<script>
function addCompositionMarker(elem, start, end) {
const range = document.createRange();
const textNode = elem.firstChild;
range.setStart(textNode, start);
range.setEnd(textNode, end);
if (typeof internals !== 'undefined')
internals.addCompositionMarker(range, 'orange', 'thin', 'lightBlue');
};
function addSpellingMarker(elem, start, end) {
const range = document.createRange();
const textNode = elem.firstChild;
range.setStart(textNode, start);
range.setEnd(textNode, end);
if (typeof internals !== 'undefined')
internals.setMarker(document, range, 'spelling');
};
function addTextMatchMarker(elem, start, end) {
const range = document.createRange();
const textNode = elem.firstChild;
range.setStart(textNode, start);
range.setEnd(textNode, end);
if (typeof internals !== 'undefined') {
internals.addTextMatchMarker(range, 'kActive');
internals.setMarkedTextMatchesAreHighlighted(document, true);
}
};
onload = runAfterLayoutAndPaint(function() {
addCompositionMarker(div, 5, 16);
addSpellingMarker(div, 17, 25);
addTextMatchMarker(div, 26, 35);
}, true);
</script>
third_party/WebKit/LayoutTests/platform/linux/paint/markers/first-letter-expected.txt 0 → 100644
View file @ abf98057
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x36
LayoutBlockFlow {HTML} at (0,0) size 800x36
LayoutBlockFlow {BODY} at (8,8) size 784x20
LayoutBlockFlow {DIV} at (0,0) size 784x20
LayoutInline {<pseudo:first-letter>} at (0,0) size 12x19 [color=#FF0000]
LayoutTextFragment (anonymous) at (0,0) size 12x19
text run at (0,0) width 12: "N"
LayoutTextFragment {#text} at (12,0) size 236x19
text run at (12,0) width 236: "one Composition Spelling TextMatch"
third_party/WebKit/LayoutTests/platform/linux/virtual/spv175/paint/markers/first-letter-expected.txt 0 → 100644
View file @ abf98057
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x36
LayoutBlockFlow {HTML} at (0,0) size 800x36
LayoutBlockFlow {BODY} at (8,8) size 784x20
LayoutBlockFlow {DIV} at (0,0) size 784x20
LayoutInline {<pseudo:first-letter>} at (0,0) size 12x19 [color=#FF0000]
LayoutTextFragment (anonymous) at (0,0) size 12x19
text run at (0,0) width 12: "N"
LayoutTextFragment {#text} at (12,0) size 236x19
text run at (12,0) width 236: "one Composition Spelling TextMatch"
third_party/WebKit/LayoutTests/platform/mac/paint/markers/first-letter-expected.txt 0 → 100644
View file @ abf98057
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x34
LayoutBlockFlow {HTML} at (0,0) size 800x34
LayoutBlockFlow {BODY} at (8,8) size 784x18
LayoutBlockFlow {DIV} at (0,0) size 784x18
LayoutInline {<pseudo:first-letter>} at (0,0) size 12x18 [color=#FF0000]
LayoutTextFragment (anonymous) at (0,0) size 12x18
text run at (0,0) width 12: "N"
LayoutTextFragment {#text} at (11,0) size 241x18
text run at (11,0) width 241: "one Composition Spelling TextMatch"
third_party/WebKit/LayoutTests/platform/win/paint/markers/first-letter-expected.txt 0 → 100644
View file @ abf98057
layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600
layer at (0,0) size 800x36
LayoutBlockFlow {HTML} at (0,0) size 800x36
LayoutBlockFlow {BODY} at (8,8) size 784x20
LayoutBlockFlow {DIV} at (0,0) size 784x20
LayoutInline {<pseudo:first-letter>} at (0,0) size 12x19 [color=#FF0000]
LayoutTextFragment (anonymous) at (0,0) size 12x19
text run at (0,0) width 12: "N"
LayoutTextFragment {#text} at (12,0) size 223x19
text run at (12,0) width 223: "one Composition Spelling TextMatch"
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp
View file @ abf98057
......@@ -39,12 +39,19 @@ namespace {
std::pair<unsigned, unsigned> GetTextMatchMarkerPaintOffsets(
const DocumentMarker& marker,
const InlineTextBox& text_box) {
// text_box.Start() returns an offset relative to the start of the layout
// object. We add the LineLayoutItem's TextStartOffset() to get a DOM offset
// (which is what DocumentMarker uses). This is necessary to get proper
// behavior with the :first-letter psuedo element.
const unsigned text_box_start =
text_box.Start() + text_box.GetLineLayoutItem().TextStartOffset();
DCHECK_EQ(DocumentMarker::kTextMatch, marker.GetType());
const unsigned start_offset = marker.StartOffset() > text_box.Start()
? marker.StartOffset() - text_box.Start()
const unsigned start_offset = marker.StartOffset() > text_box_start
? marker.StartOffset() - text_box_start
: 0U;
const unsigned end_offset =
std::min(marker.EndOffset() - text_box.Start(), text_box.Len());
std::min(marker.EndOffset() - text_box_start, text_box.Len());
return std::make_pair(start_offset, end_offset);
}
......@@ -440,10 +447,17 @@ InlineTextBoxPainter::PaintOffsets InlineTextBoxPainter::MarkerPaintStartAndEnd(
DCHECK(inline_text_box_.Truncation() != kCFullTruncation);
DCHECK(inline_text_box_.Len());
// inline_text_box_.Start() returns an offset relative to the start of the
// layout object. We add the LineLayoutItem's TextStartOffset() to get a DOM
// offset (which is what DocumentMarker uses). This is necessary to get proper
// behavior with the :first-letter psuedo element.
const unsigned inline_text_box_start =
inline_text_box_.Start() +
inline_text_box_.GetLineLayoutItem().TextStartOffset();
// Start painting at the beginning of the text or the specified underline
// start offset, whichever is greater.
unsigned paint_start =
std::max(inline_text_box_.Start(), marker.StartOffset());
unsigned paint_start = std::max(inline_text_box_start, marker.StartOffset());
// Cap the maximum paint start to the last character in the text box.
paint_start = std::min(paint_start, inline_text_box_.end());
......@@ -455,8 +469,8 @@ InlineTextBoxPainter::PaintOffsets InlineTextBoxPainter::MarkerPaintStartAndEnd(
// paint_start and paint_end are currently relative to the start of the text
// node. Subtract to make them relative to the start of the InlineTextBox.
paint_start -= inline_text_box_.Start();
paint_end -= inline_text_box_.Start();
paint_start -= inline_text_box_start;
paint_end -= inline_text_box_start;
return ApplyTruncationToPaintOffsets({paint_start, paint_end});
}
......
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