Issue 5542: Implement tooltips for new options page

desktop-options.js

Index: desktop-options.js
===================================================================
--- a/desktop-options.js
+++ b/desktop-options.js
@@ -1295,7 +1295,7 @@
         updateSubscription(subscription);
         break;
       case "added":
-        let {url, recommended} = subscription;
+        let {url} = subscription;
         // Handle custom subscription
         if (/^~user/.test(url))
         {
@@ -1401,30 +1401,42 @@
 
   function updateTooltips()
   {
-    let anchors = document.querySelectorAll(":not(.tooltip) > [data-tooltip]");
-    for (let anchor of anchors)
+    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)
     {
       let id = anchor.getAttribute("data-tooltip");
 
-      let wrapper = document.createElement("div");
-      wrapper.setAttribute("aria-describedby", id);
-      wrapper.setAttribute("tabindex", "0");
-      wrapper.className = "icon tooltip";
-      anchor.parentNode.replaceChild(wrapper, anchor);
-      wrapper.appendChild(anchor);
+      // 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");
 
-      let tooltip = document.createElement("div");
-      tooltip.id = id;
+      const tooltip = document.createElement("div");
       tooltip.setAttribute("role", "tooltip");
+      // needed by aria-describedby
+      tooltip.id = id;
 
-      let paragraph = document.createElement("p");
+      const paragraph = document.createElement("p");
       paragraph.textContent = getMessage(id);
       tooltip.appendChild(paragraph);
 
-      wrapper.appendChild(tooltip);
+      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;
+    if (active)
+      active.blur();
+  }
+
   ext.onMessage.addListener((message) =>
   {
     switch (message.type)