From e1df166204ad9dc87605184e89ae91fd2db23de3 Mon Sep 17 00:00:00 2001
From: Daniel Freedman <dfreedm@google.com>
Date: Wed, 21 Feb 2018 16:43:36 -0800
Subject: [PATCH] Add docs and cleanup matchingLabels

Remove broken `while` loop, as that case is already handled by the
mouseCancellor code
---
 lib/utils/gestures.html | 20 ++++++++------------
 test/unit/gestures.html |  7 +++++--
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/utils/gestures.html b/lib/utils/gestures.html
index 94cd964dbd..ca38ec3173 100644
--- a/lib/utils/gestures.html
+++ b/lib/utils/gestures.html
@@ -128,17 +128,13 @@
   function matchingLabels(el) {
     /** @type {!Array<!HTMLLabelElement>} */
     let labels = el.labels;
-    // IE doesn't have `labels` and Safari doesn't populate `labels` if element is in a shadowroot.
+    // IE doesn't have `labels` and Safari doesn't populate `labels`
+    // if element is in a shadowroot.
+    // In this instance, finding the non-ancestor labels is enough,
+    // as the mouseCancellor code will handle ancstor labels
     if (!labels || !labels.length) {
       labels = [];
       let root = el.getRootNode();
-      // find all labels that are ancestors
-      let cur = el.parentNode;
-      while (cur !== root) {
-        if (cur.localName === 'label') {
-          labels.push(/** @type {!HTMLLabelElement} */(cur));
-        }
-      }
       // if there is an id on `el`, check for all labels with a matching `for` attribute
       if (el.id) {
         let matching = root.querySelectorAll(`label[for = ${el.id}]`);
@@ -168,28 +164,28 @@
     mouseEvent[HANDLED_OBJ] = {skip: true};
     // disable "ghost clicks"
     if (mouseEvent.type === 'click') {
-      let labels = [];
       let clickFromLabel = false;
       let path = mouseEvent.composedPath && mouseEvent.composedPath();
       if (path) {
         for (let i = 0; i < path.length; i++) {
           if (path[i].nodeType === Node.ELEMENT_NODE) {
             if (path[i].localName === 'label') {
-              labels.push(path[i]);
+              clickedLabels.push(path[i]);
             } else if (canBeLabelled(path[i])) {
               let ownerLabels = matchingLabels(path[i]);
+              // check if one of the clicked labels is labelling this element
               for (let j = 0; j < ownerLabels.length; j++) {
                 clickFromLabel = clickFromLabel || clickedLabels.indexOf(ownerLabels[j]) > -1;
               }
             }
           }
           if (path[i] === POINTERSTATE.mouse.target) {
-            // commit tracked labels to "clickedLabels"
-            clickedLabels.push(...labels);
             return;
           }
         }
       }
+      // if one of the clicked labels was labelling the target element,
+      // this is not a ghost click
       if (clickFromLabel) {
         return;
       }
diff --git a/test/unit/gestures.html b/test/unit/gestures.html
index b3ad058953..465abf2fe6 100644
--- a/test/unit/gestures.html
+++ b/test/unit/gestures.html
@@ -564,11 +564,12 @@
         let el = document.createElement('x-native-label');
         document.body.appendChild(el);
         let target = el.$.label;
+        // simulate the event sequence of a touch on the screen
         let touches = [{
           clientX: 0,
           clientY: 0,
           identifier: 1,
-          // target is set to the element with `addEventListener`, which is app
+          // target is set to the element with `addEventListener`, which is `target`
           target
         }];
         let touchstart = new CustomEvent('touchstart', {bubbles: true, composed: true});
@@ -577,9 +578,11 @@
         let touchend = new CustomEvent('touchend', {bubbles: true, composed: true});
         touchend.touches = touchend.changedTouches = touches;
         target.dispatchEvent(touchend);
+        // simulate a mouse click on the label
         let click = new MouseEvent('click', {bubbles: true, composed: true});
         target.dispatchEvent(click);
-        assert.equal(el.$.check.checked, true);
+        // check that the mouse click on the label will activate the checkbox
+        assert.equal(el.$.check.checked, true, 'checkbox should be checked');
         document.body.removeChild(el);
       });
     });