Commit bd25c6aa authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Fix hit-testing image list markers

This patch fixes hit-testing image list markers. The previous
fix for hit-testing list markers (r682470 crrev.com/c/1726571)
didn't cover image list markers, and unfortunately the test
did not cover the case.

Bug: 1002417
Change-Id: Ieefcfa91240570a3d59265cf7e75c802396da6d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862873Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706094}
parent 7084d3cd
...@@ -22,6 +22,21 @@ bool LayoutNGListMarkerImage::IsOfType(LayoutObjectType type) const { ...@@ -22,6 +22,21 @@ bool LayoutNGListMarkerImage::IsOfType(LayoutObjectType type) const {
return type == kLayoutObjectNGListMarkerImage || LayoutImage::IsOfType(type); return type == kLayoutObjectNGListMarkerImage || LayoutImage::IsOfType(type);
} }
Node* LayoutNGListMarkerImage::NodeForHitTest() const {
// In LayoutNG tree, image list marker is structured like this:
// <li> (LayoutListItem)
// <anonymous block> (LayoutNGListMarker or LayoutNGInsideListMarker)
// <anonymous img> (LayoutNGListMarkerImage)
// Hit testing should return the list-item node.
DCHECK(!GetNode());
for (const LayoutObject* parent = Parent(); parent;
parent = parent->Parent()) {
if (Node* node = parent->GetNode())
return node;
}
return nullptr;
}
// Because ImageResource() is always LayoutImageResourceStyleImage. So we could // Because ImageResource() is always LayoutImageResourceStyleImage. So we could
// use StyleImage::ImageSize to determine the concrete object size with // use StyleImage::ImageSize to determine the concrete object size with
// default object size(ascent/2 x ascent/2). // default object size(ascent/2 x ascent/2).
......
...@@ -19,6 +19,8 @@ class CORE_EXPORT LayoutNGListMarkerImage final : public LayoutImage { ...@@ -19,6 +19,8 @@ class CORE_EXPORT LayoutNGListMarkerImage final : public LayoutImage {
bool IsLayoutNGObject() const override { return true; } bool IsLayoutNGObject() const override { return true; }
Node* NodeForHitTest() const final;
private: private:
bool IsOfType(LayoutObjectType) const override; bool IsOfType(LayoutObjectType) const override;
......
...@@ -5,28 +5,79 @@ ...@@ -5,28 +5,79 @@
<script src="/resources/testharness.js"></script> <script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script> <script src="/resources/testharnessreport.js"></script>
<style> <style>
ul {
font-size: 10px;
}
ul.inside { ul.inside {
list-style-position: inside; list-style-position: inside;
} }
.image {
list-style-image: url(../../images/green-16x16.png);
}
.debug-marker {
position: absolute;
width: 0;
height: 0;
outline: 2px solid red;
}
</style> </style>
<body> <body>
<ul> <ul>
<li>Outside 1</li> <li>Outside 1</li>
<li>Outside 2</li> <li>Outside 2</li>
<li>Outside 3</li> <li>Outside 3</li>
<li class="image">Image Outside 1</li>
<li class="image">Image Outside 2</li>
</ul> </ul>
<ul class="inside"> <ul class="inside">
<li>Inside 1</li> <li>Inside 1</li>
<li>Inside 2</li> <li>Inside 2</li>
<li>Inside 3</li> <li>Inside 3</li>
<li class="image">Image Inside 1</li>
<li class="image">Image Inside 2</li>
</ul> </ul>
<script> <script>
for (let li of document.getElementsByTagName('li')) { setup({explicit_done:true});
test(() => { window.onload = function() {
let bounds = li.getBoundingClientRect(); for (let li of document.getElementsByTagName('li')) {
let target = document.elementFromPoint(bounds.x + 1, bounds.y + 1); test(() => {
assert_equals(target, li); let bounds = li.getBoundingClientRect();
}, `<li>${li.textContent}</li>`); let style = window.getComputedStyle(li);
let y = (bounds.top + bounds.bottom) / 2;
if (style.listStylePosition === 'inside') {
// Inside markers are normal inline boxes.
// It is safe to assume "left + 1" is in the box.
let x = bounds.left + 1;
addDebugMarker(x, y);
let result = document.elementFromPoint(x, y);
assert_equals(result, li);
} else {
// The spec does not define outside marker position.
// This code assumes that the marker is within "left - 40" to "left - 1".
// This is heuristic, but all browsers seem to pass with this method.
let result = null;
for (let x = bounds.left - 40; x < bounds.left; x++) {
result = document.elementFromPoint(x, y);
if (result === li) {
addDebugMarker(x, y);
break;
}
}
assert_equals(result, li);
}
}, `<li>${li.textContent}</li>`);
}
done();
};
// Show a marker for the debugging purposes, in case the heuristic doesn't apply.
function addDebugMarker(x, y) {
let div = document.createElement('div');
div.className = 'debug-marker';
let style = div.style;
style.left = x + 'px';
style.top = y + 'px';
document.body.appendChild(div);
} }
</script> </script>
</body> </body>
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