Issue 5542: Implement tooltips for new options page

desktop-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 515 matching lines @@
                                             type + "_title");
           }
 
           addSubscription(subscription);
         }
       });
   }
 
   function findParentData(element, dataName, returnElement)
   {
-    element = element.closest(`[data-${dataName}]`);
-    if (!element)
-      return null;
-    if (returnElement)
-      return element;
-    return element.getAttribute(`data-${dataName}`);
+    while (element)
+    {
+      if (element.hasAttribute("data-" + dataName))
+      {
+        if (returnElement)
+          return element;
+        return element.getAttribute("data-" + dataName);
+      }
+
+      element = element.parentElement;
+    }
+    return null;
   }
 
   function sendMessageHandleErrors(message, onSuccess)
   {
     browser.runtime.sendMessage(message, (errors) =>
     {
       if (errors.length > 0)
         alert(errors.join("\n"));
       else if (onSuccess)
         onSuccess();
@@ skipping 836 matching lines @@
 
     let checkbox = document.querySelector(
       "[data-pref='" + key + "'] button[role='checkbox']"
     );
     if (checkbox)
       checkbox.setAttribute("aria-checked", value);
   }
 
   function updateTooltips()
   {
-    const tooltipsSelector = ":not(.tooltip) > [data-tooltip]";
-    const anchors = document.querySelectorAll(tooltipsSelector);
+    const anchors = document.querySelectorAll("[data-tooltip]");

[sergei, 2018/02/19 10:18:42]

I thought we don't use `const` in such cases, do we?

[a.giammarchi, 2018/02/19 10:26:38]

why not, if I might ask? `const` is used to guard against undesired
reassignments. It's true that in this file I actually should drop all `let` and
use `const` instead, but for new code, where it makes sense, I don't see why we
shouldn't use language guards/features.

If this was explicitly written in some code guideline thought, I'd be happy to
stick with it and preserve consistency. Yet I don't understand why would we make
such decision.

Thanks in advance for any sort of clarification.

[sergei, 2018/02/19 10:44:45]

Yep, I know what it is for but I got several times in codereviwes that we should
use `let` in such cases. That's because I'm asking to know the current state.
I find the wording actually vague
https://adblockplus.org/en/coding-style#javascript
"> Use const for constant expressions that could as well have been inlined (e.g.
static parameters, imported modules, etc.)."

[a.giammarchi, 2018/02/19 10:53:05]

that's the second point though, the first one says:
The for/of is block-scoping so I think I've followed the rules.

     for (const anchor of anchors)
     {
-      // avoid creating same tooltip N times
-      // updateTooltips gets invoked during bootstrap
-      if (anchor.querySelector('[role="tooltip"]'))
-        continue;
-
-      const id = anchor.getAttribute("data-tooltip");
-
+      let id = anchor.getAttribute("data-tooltip");
+
+      // instead of replacing the whole node
+      // just make it non discoverable for
+      // the next updateTooltips call
+      anchor.removeAttribute("data-tooltip");
       anchor.classList.add("icon", "tooltip");
 
       const tooltip = document.createElement("div");
       tooltip.setAttribute("role", "tooltip");
+      // needed by aria-describedby
+      tooltip.id = id;
 
       const paragraph = document.createElement("p");
       paragraph.textContent = getMessage(id);
       tooltip.appendChild(paragraph);
 
       anchor.appendChild(tooltip);
+      anchor.setAttribute("aria-describedby", id);
 
       // if focused and the mouse reaches another (?)
       // blur the active element to drop previous tooltip
       anchor.addEventListener("mouseover", dropActiveElement);
     }
   }
 
   function dropActiveElement(event)
   {
     const active = event.target.ownerDocument.activeElement;
@@ skipping 48 matching lines @@
   });
   browser.runtime.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }