Issue 2879 - Move element selection into the content process

lib/child/commands.js

+/*
+ * This Source Code is subject to the terms of the Mozilla Public License
+ * version 2.0 (the "License"). You can obtain a copy of the License at
+ * http://mozilla.org/MPL/2.0/.
+ */
+
+"use strict";
+
+let messageManager = require("messageManager");
+let {getNodeInfo} = require("./nodeInfo");
+let {
+  state, selectElement, setAnchorElement, stopSelection
+} = require("./selection");
+let {getParentElement} = require("./utils");
+
+messageManager.addMessageListener("ElemHideHelper:Command", onCommand);
+
+onShutdown.add(() =>
+{
+  messageManager.removeMessageListener("ElemHideHelper:Command", onCommand);
+});
+
+function onCommand(message)
+{
+  let command = message.data;
+  if (typeof exports[command] == "function")
+    exports[command]();
+}
+
+function quit()
+{
+  stopSelection();
+}
+exports.quit = quit;
+
+function select()
+{
+  if (!state.selectedElement)
+    return;
+
+  messageManager.sendAsyncMessage(
+    "ElemHideHelper:SelectionSucceeded",
+    getNodeInfo(state.selectedElement)
+  );
+  stopSelection();

[saroyanm, 2016/11/23 17:44:38]

Why not call stopSelection in SelectionSucceeded method, IMO that should be
handled by that method, so we do not need call it each time we are calling
SelectionSucceeded event.

[Wladimir Palant, 2016/11/24 14:02:01]

stopSelection() is a general cleanup method. It doesn't merely send the
SelectionStopped message so that the parent knows it needs to cleanup, it also
performs some cleanup in the child - like removing event handlers. It actually
makes more sense to have only one cleanup path.

+}
+exports.select = select;
+
+function wider()
+{
+  if (!state.selectedElement)
+    return;
+
+  let newElement = getParentElement(state.selectedElement);
+  if (!newElement)
+    return;
+
+  state.isUserSelected = true;
+  selectElement(newElement);
+}
+exports.wider = wider;
+
+function narrower()
+{
+  if (!state.selectedElement)
+    return;
+
+  // Search selected element in the parent chain, starting with the ancho
+  // element. We need to select the element just before the selected one.
+  let e = state.anchorElement;
+  let newElement = null;
+  while (e && e != state.selectedElement)
+  {
+    newElement = e;
+    e = getParentElement(e);
+  }
+
+  if (!e || !newElement)
+    return;
+
+  state.isUserSelected = true;
+  selectElement(newElement);
+}
+exports.narrower = narrower;
+
+function lock()
+{
+  if (!state.selectedElement)
+    return;
+
+  if (state.lockedAnchor)
+  {
+    setAnchorElement(state.lockedAnchor);
+    state.lockedAnchor = null;
+  }
+  else
+    state.lockedAnchor = state.anchorElement;
+}
+exports.lock = lock;
+
+let blinkState = null;

[Wladimir Palant, 2016/11/17 13:54:48]

I moved the properties related to the blink functionality into their own object
in order to achieve code locality - everything related to blinking is located
right here. One consequence is: canceling element selection will no longer
cancel blinking. Given that blinking stops after 1.5 seconds anyway this doesn't
seem to be an issue. However, I had to add checks for the case where the page
unloads while we are still blinking (look for Cu.isDeadWrapper). This might not
have been an issue before because pagehide event cancels selection meaning that
blinking would be canceled as well.

[saroyanm, 2016/11/23 17:44:38]

Acknowledged.

+
+function stopBlinking()
+{
+  blinkState.timer.cancel();
+  if (!Cu.isDeadWrapper(blinkState.element))
+    blinkState.element.style.visibility = blinkState.origVisibility;
+  blinkState = null;
+}
+
+function doBlink()

[Wladimir Palant, 2016/11/17 13:54:48]

This was a closure in the original code, having it like that with the entire
state stored globally seemed pointless however.

[saroyanm, 2016/11/23 17:44:38]

Acknowledged.

+{
+  if (Cu.isDeadWrapper(blinkState.element))
+  {
+    stopBlinking();
+    return;
+  }
+
+  blinkState.counter++;
+  blinkState.element.style.setProperty(
+    "visibility",
+    (blinkState.counter % 2 == 0 ? "visible" : "hidden"),
+    "important"
+  );

[Wladimir Palant, 2016/11/17 13:54:48]

Original code was simply assigning element.style.visibility, I added !important
here.

[saroyanm, 2016/11/23 17:44:38]

Acknowledged.

+  if (blinkState.counter == 6)
+    stopBlinking();
+}
+
+function blinkElement()
+{
+  if (!state.selectedElement)
+    return;
+
+  if (blinkState)
+    stopBlinking();
+
+  blinkState = {
+    counter: 0,
+    element: state.selectedElement,
+    origVisibility: state.selectedElement.style.visibility,
+    timer: Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer)
+  };
+
+  blinkState.timer.initWithCallback(doBlink, 250,
+      Ci.nsITimer.TYPE_REPEATING_SLACK);
+}
+exports.blinkElement = blinkElement;