[LayoutNG] Fix hit-testing image list markers

kojiishi · chromium-wpt-export-bot · commit 01fc47690771 · 2019-10-15T16:14:48.000-04:00
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/+/1862873 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#706094}
diff --git a/css/cssom-view/elementFromPoint-list-001.html b/css/cssom-view/elementFromPoint-list-001.html
@@ -5,28 +5,79 @@
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <style>
+ul {
+  font-size: 10px;
+}
 ul.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>
 <body>
   <ul>
     <li>Outside 1</li>
     <li>Outside 2</li>
     <li>Outside 3</li>
+    <li class="image">Image Outside 1</li>
+    <li class="image">Image Outside 2</li>
   </ul>
   <ul class="inside">
     <li>Inside 1</li>
     <li>Inside 2</li>
     <li>Inside 3</li>
+    <li class="image">Image Inside 1</li>
+    <li class="image">Image Inside 2</li>
   </ul>
 <script>
-for (let li of document.getElementsByTagName('li')) {
-  test(() => {
-    let bounds = li.getBoundingClientRect();
-    let target = document.elementFromPoint(bounds.x + 1, bounds.y + 1);
-    assert_equals(target, li);
-  }, `<li>${li.textContent}</li>`);
+setup({explicit_done:true});
+window.onload = function() {
+  for (let li of document.getElementsByTagName('li')) {
+    test(() => {
+      let bounds = li.getBoundingClientRect();
+      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>
 </body>
