From 46714d39d7cc988b9659fae2caf6093a2686c609 Mon Sep 17 00:00:00 2001
From: Jonathan Wilkes <jon.w.wilkes@gmail.com>
Date: Fri, 23 Jun 2017 16:58:37 -0400
Subject: [PATCH] guard against Chromium's counter-intuitive
 mouseenter/mouseover/mousemove events that get generated when a scroll event
 moves the element under the mouse Also, use "mousemove" instead of
 "mouseover" event as it properly highlights the element under the mouse even
 for single pixel mouse movements. "mouseover" would fail to highlight if the
 keyboard navigation had moved the highlight to a different element within
 view. These changes fix the problem of generating a chain of mouse events
 that would mess up keyboard navigation when the mouse pointer happened to be
 over the <ol>. These changes make the interaction between mouse and keyboard
 much more like what's expected in most GUI toolkits. (The only difference now
 is that the menu can't overflow the browser window, but there's nothing we
 can do about that.)

---
 pd/nw/pd_canvas.js | 64 ++++++++++++++++++++++++++++++++++------------
 pd/nw/pdgui.js     | 61 +++++++++++++++++++++++++++++++------------
 2 files changed, 92 insertions(+), 33 deletions(-)

diff --git a/pd/nw/pd_canvas.js b/pd/nw/pd_canvas.js
index 6eeabd766..fb290e298 100644
--- a/pd/nw/pd_canvas.js
+++ b/pd/nw/pd_canvas.js
@@ -160,6 +160,8 @@ var canvas_events = (function() {
         last_draggable_y,       // last y
         previous_state = "none", /* last state, excluding explicit 'none' */
         match_words_state = false,
+        last_dropdown_menu_x,
+        last_dropdown_menu_y,
         last_search_term = "",
         svg_view = document.getElementById("patchsvg").viewBox.baseVal,
         textbox = function () {
@@ -211,7 +213,7 @@ var canvas_events = (function() {
             pdgui.pdsend(elem.getAttribute("data-callback"),
                 elem.querySelector(".highlighted").getAttribute("data-index"));
         },
-        dropdown_highlight_elem = function(elem) {
+        dropdown_highlight_elem = function(elem, scroll) {
             var container = document.querySelector("#dropdown_list"),
                 li_array;
             if (!elem.classList.contains("highlighted")) {
@@ -221,8 +223,25 @@ var canvas_events = (function() {
                 });
                 elem.classList.add("highlighted");
                 // Make sure the highlighted element is in view
-                container.scrollTop = elem.offsetTop + elem.offsetHeight
-                    - container.clientHeight;
+                if (scroll) {
+                    if (elem.offsetTop < container.scrollTop
+                        || elem.offsetTop + elem.offsetHeight >
+                            container.scrollTop + container.offsetHeight) {
+                            if (scroll === "up") {
+                                // we can't usse elem.scrollIntoView() here
+                                // because it may also change the scrollbar on
+                                // the document, which in turn could change the
+                                // pageX/pageY, for the mousemove event, which
+                                // in turn would make it impossible for us to
+                                // filter out unnecessary mousemove calls
+                                //    elem.scrollIntoView();
+                            container.scrollTop = elem.offsetTop;
+                        } else if (scroll === "down") {
+                            container.scrollTop = elem.offsetTop + elem.offsetHeight
+                                - container.clientHeight;
+                        }
+                    }
+                }
             }
         },
         events = {
@@ -494,13 +513,13 @@ var canvas_events = (function() {
             iemgui_label_mouseup: function(evt) {
                 //pdgui.post("lifting the mousebutton on an iemgui label");
                 // Set last state (none doesn't count as a state)
-                //pdgui.post("previous state is " + canvas_events.get_previous_state());
+                //pdgui.post("previous state is "
+                //    + canvas_events.get_previous_state());
                 canvas_events[canvas_events.get_previous_state()]();
             },
             dropdown_menu_keydown: function(evt) {
                 var select_elem = document.querySelector("#dropdown_list"),
                     li;
-                pdgui.post("keycode is " + evt.keyCode);
                 switch(evt.keyCode) {
                     case 13:
                     case 32:
@@ -517,17 +536,18 @@ var canvas_events = (function() {
                         li = select_elem.querySelector(".highlighted");
                         li = li.previousElementSibling ||
                              li.parentElement.lastElementChild;
-                        dropdown_highlight_elem(li);
+                        dropdown_highlight_elem(li, "up");
+                        evt.preventDefault();
                         break;
                     case 40: // down
                         li = select_elem.querySelector(".highlighted");
                         li = li.nextElementSibling ||
                              li.parentElement.firstElementChild;
-                        dropdown_highlight_elem(li);
+                        dropdown_highlight_elem(li, "down");
+                        evt.preventDefault();
                         break;
                     default:
                 }
-                evt.preventDefault();
             },
             dropdown_menu_keypress: function(evt) {
                 var li_nodes = document.querySelectorAll("#dropdown_list li"),
@@ -551,7 +571,8 @@ var canvas_events = (function() {
                 if (match !== undefined) {
                     match = (match + offset) % li_nodes.length;
                     if (match !== highlighted_index) {
-                        dropdown_highlight_elem(li_nodes[match]);
+                        dropdown_highlight_elem(li_nodes[match],
+                            match < highlighted_index ? "up" : "down");
                     }
                 }
             },
@@ -589,14 +610,25 @@ var canvas_events = (function() {
                     canvas_events.normal();
                 }
             },
-            dropdown_menu_mouseover: function(evt) {
+            dropdown_menu_mousemove: function(evt) {
                 var li_array;
-                if (evt.target.parentNode
-                    && evt.target.parentNode.parentNode
-                    && evt.target.parentNode.parentNode.id === "dropdown_list") {
-                    dropdown_highlight_elem(evt.target);
+                // For whatever reason, Chromium decides to trigger the
+                // mousemove/mouseenter/mouseover events if the element
+                // underneath it changes (or for mousemove, if the element
+                // moves at all). Unfortunately that means we have to track
+                // the mouse position the entire time to filter out the
+                // true mousemove events from the ones Chromium generates
+                // when a scroll event changes the element under the mouse.
+                if (evt.pageX !== last_dropdown_menu_x
+                    || evt.pageY !== last_dropdown_menu_y) {
+                    if (evt.target.parentNode
+                        && evt.target.parentNode.parentNode
+                        && evt.target.parentNode.parentNode.id === "dropdown_list") {
+                        dropdown_highlight_elem(evt.target);
+                    }
                 }
-                // hide the dropdown menu <div> thingy
+                last_dropdown_menu_x = evt.pageX;
+                last_dropdown_menu_y = evt.pageY;
             }
         },
         utils = {
@@ -853,7 +885,7 @@ var canvas_events = (function() {
             this.none();
             document.addEventListener("mousedown", events.dropdown_menu_mousedown, false);
             document.addEventListener("mouseup", events.dropdown_menu_mouseup, false);
-            document.addEventListener("mouseover", events.dropdown_menu_mouseover, false);
+            document.addEventListener("mousemove", events.dropdown_menu_mousemove, false);
             document.addEventListener("keydown", events.dropdown_menu_keydown, false);
             document.addEventListener("keypress", events.dropdown_menu_keypress, false);
         },
diff --git a/pd/nw/pdgui.js b/pd/nw/pdgui.js
index 657ce1218..0e6f42526 100644
--- a/pd/nw/pdgui.js
+++ b/pd/nw/pdgui.js
@@ -4596,7 +4596,13 @@ function dropdown_populate(cid, label_array, current_index) {
 }
 
 function gui_dropdown_activate(cid, obj_tag, tag, current_index, font_size, state, label_array) {
-    var g, select_elem, svg_view;
+    var g, select_elem, svg_view, g_bbox,
+        w_height,    // excluding the scrollbar
+        menu_height, // height of the list of elements inside the div
+        div_y,       // position of div containing the dropdown menu
+        div_max,     // max height of the div
+        scroll_y,
+        offset_anchor; // top or bottom
     // Annoying: obj_tag is just the "x"-prepended hex value for the object,
     // and tag is the one from rtext_gettag that is used as our gobj id
     if (patchwin[cid]) {
@@ -4604,28 +4610,49 @@ function gui_dropdown_activate(cid, obj_tag, tag, current_index, font_size, stat
         if (state !== 0) {
             svg_view = patchwin[cid].window.document.getElementById("patchsvg")
                 .viewBox.baseVal;
-            dropdown_populate(cid, label_array, current_index);
             select_elem = patchwin[cid]
                 .window.document.querySelector("#dropdown_list");
+            select_elem.style.setProperty("display", "inline");
+            dropdown_populate(cid, label_array, current_index);
             // stick the obj_tag in a data field
             select_elem.setAttribute("data-callback", obj_tag);
-            // set the maximum height of the menu to be the remaining
-            // space below the corresponding widget, minus the size of
-            // the scrollbar. (innerHeight includes scrollbar, and
-            // the documentElement's clientHeight (apparently) does not.
-            select_elem.style.setProperty("max-height",
-                (patchwin[cid].window.innerHeight -
-                    g.getBoundingClientRect().bottom -
-                    (patchwin[cid].window.innerHeight -
-                    patchwin[cid].window.document.documentElement.clientHeight))
-                    + "px"
-            );
-            select_elem.style.setProperty("display", "inline");
+            // display the menu so we can measure it
+
+            g_bbox = g.getBoundingClientRect();
+            w_height =
+                patchwin[cid].window.document.documentElement
+                    .clientHeight;
+            menu_height = select_elem.querySelector("ol")
+                .getBoundingClientRect().height;
+            scroll_y = patchwin[cid].window.scrollY;
+            // If the area below the object is smaller than 75px, then
+            // display the menu above the object.
+            // If the entire menu won't fit below the object but _will_
+            // fit above it, display it above the object.
+            // If the menu needs a scrollbar, display it below the object
+            if (w_height - g_bbox.bottom <= 75
+                || (menu_height > w_height - g_bbox.bottom
+                    && menu_height <= g_bbox.top)) {
+                // menu on top
+                offset_anchor = "bottom";
+                div_max = g_bbox.top - 1;
+                div_y = w_height - (g_bbox.top + scroll_y);
+            }
+            else {
+                // menu on bottom (possibly with scrollbar)
+                offset_anchor = "top";
+                div_max = w_height - g_bbox.bottom - 1;
+                div_y = g_bbox.bottom + scroll_y;
+            }
+            // set a max-height to force scrollbar if needed
+            select_elem.style.setProperty("max-height", div_max + "px");
             select_elem.style.setProperty("left",
                 (elem_get_coords(g).x - svg_view.x) + "px");
-            select_elem.style.setProperty("top",
-                (elem_get_coords(g).y + g.getBBox().height - svg_view.y)
-                    + "px");
+            // Remove "top" and "bottom" props to keep state clean
+            select_elem.style.removeProperty("top");
+            select_elem.style.removeProperty("bottom");
+            // Now position the div relative to either the "top" or "bottom"
+            select_elem.style.setProperty(offset_anchor, div_y + "px");
             select_elem.style.setProperty("font-size",
                 pd_fontsize_to_gui_fontsize(font_size) + "px");
             select_elem.style.setProperty("min-width", g.getBBox().width + "px");
-- 
GitLab