Commit acdd73ae authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

[LayoutNG] Work around crash with 'text-transform'

'text-transform' may make LayoutText::GetText() longer than the original
DOM text node, and result in out-of-range offsets that are not properly
handled.

This patch works around that in some crash sites so that it stops
crashing.

Bug: 899868
Change-Id: I55645b83b3d2d0aec38cb2c0f97a2b89a4897d3e
Reviewed-on: https://chromium-review.googlesource.com/c/1450745
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628916}
parent 85ddefea
......@@ -2174,7 +2174,13 @@ Position LayoutText::PositionForCaretOffset(unsigned offset) const {
return Position();
DCHECK(node->IsTextNode());
// TODO(layout-dev): Support offset change due to text-transform.
return Position(node, offset);
#if DCHECK_IS_ON()
// Ensures that the clamping hack kicks in only with text-transform.
if (StyleRef().TextTransform() == ETextTransform::kNone)
DCHECK_LE(offset, ToText(node)->length());
#endif
const unsigned clamped_offset = std::min(offset, ToText(node)->length());
return Position(node, clamped_offset);
}
base::Optional<unsigned> LayoutText::CaretOffsetForPosition(
......
......@@ -6,6 +6,7 @@
#include <algorithm>
#include "third_party/blink/renderer/core/dom/node.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/dom/text.h"
#include "third_party/blink/renderer/core/editing/editing_utilities.h"
#include "third_party/blink/renderer/core/editing/ephemeral_range.h"
......@@ -34,8 +35,18 @@ bool IsNonAtomicInline(const Node& node) {
}
Position CreatePositionForOffsetMapping(const Node& node, unsigned dom_offset) {
if (node.IsTextNode())
return Position(&node, dom_offset);
if (node.IsTextNode()) {
// 'text-transform' may make the rendered text length longer than the
// original text node, in which case we clamp the offset to avoid crashing.
// TODO(crbug.com/750990): Support 'text-transform' to remove this hack.
#if DCHECK_IS_ON()
// Ensures that the clamping hack kicks in only with text-transform.
if (node.ComputedStyleRef().TextTransform() == ETextTransform::kNone)
DCHECK_LE(dom_offset, ToText(node).length());
#endif
const unsigned clamped_offset = std::min(dom_offset, ToText(node).length());
return Position(&node, clamped_offset);
}
// For non-text-anchored position, the offset must be either 0 or 1.
DCHECK_LE(dom_offset, 1u);
return dom_offset ? Position::AfterNode(node) : Position::BeforeNode(node);
......
......@@ -33,6 +33,7 @@ crbug.com/591099 fast/css3-text/css3-text-justify/text-justify-distribute.html [
# New passes
crbug.com/774229 editing/pasteboard/copy-paste-white-space.html [ Pass ]
crbug.com/899868 editing/selection/mouse/drag_over_text_transform.html [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/floats-clear/no-clearance-adjoining-opposite-float.html [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/floats-clear/no-clearance-due-to-large-margin-after-left-right.html [ Pass ]
crbug.com/877946 external/wpt/css/CSS2/linebox/anonymous-inline-inherit-001.html [ Pass ]
......
......@@ -643,6 +643,7 @@ crbug.com/591099 external/wpt/css/css-ui/text-overflow-015.html [ Failure ]
# Tests for LayoutNG to pass, but not planned to fix for pre-LayoutNG.
crbug.com/774229 editing/pasteboard/copy-paste-white-space.html [ Failure ]
crbug.com/899868 editing/selection/mouse/drag_over_text_transform.html [ Crash ]
crbug.com/855279 fast/css/text-overflow-ellipsis-vertical-hittest.html [ Failure ]
crbug.com/591099 fast/css-intrinsic-dimensions/height-positioned.html [ Failure ]
crbug.com/899902 fast/text/ellipsis-with-self-painting-layer.html [ Failure ]
......
<!DOCTYPE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<style>
#target { text-transform: uppercase; }
</style>
<p id="target">&#xDF;abc</p>
<script>
// Drag through text with 'text-transform'. Shouldn't crash.
function dragThrough(x1, x2, y) {
return new Promise((resolve, reject) => {
assert_own_property(window, 'chrome');
assert_own_property(window.chrome, 'gpuBenchmarking');
chrome.gpuBenchmarking.pointerActionSequence([{
source: 'mouse',
actions: [
{name: 'pointerDown', x: x1, y: y},
{name: 'pointerMove', x: x2, y:y},
{name: 'pointerUp'}
]}], resolve);
});
}
promise_test(async () => {
const target = document.getElementById('target');
const x1 = target.offsetLeft;
const x2 = x1 + target.offsetWidth;
const y = target.offsetTop + target.offsetHeight;
await dragThrough(x1, x2, y);
const selection = getSelection();
const text = target.firstChild;
assert_false(selection.isCollapsed);
assert_equals(selection.anchorNode, text);
assert_equals(selection.anchorOffset, 0);
assert_equals(selection.focusNode, text);
assert_equals(selection.focusOffset, 4);
});
</script>
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