Bug 1493659 - Skip the click event handler if touch event has occurred. r=birtles
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Sun, 07 Oct 2018 17:52:11 +0000
changeset 495672 f34e414cd0aaa009f5fdda33f0a5a9954faee84e
parent 495671 3af0f8d207a2efe6537c8fbd2f9b15bdda05d914
child 495676 fb8edc9e0e00097813febdb64a64c82666c812be
child 495677 bfff89962ce617a7436b74503482b8b5ffff3986
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1493659
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493659 - Skip the click event handler if touch event has occurred. r=birtles Tapping the menu button during the panel has shown make the menu button to be unopenable. This cause reason is removing the pointer-events property before the mouse event happens. The tapping event will happen many the event like the following: 1) touchstart 2) touchmove (Long tap only) 3) touchend 4) mousedown 5) mouseup 6) click This patch will detect the touchend event to eat the click event. Differential Revision: https://phabricator.services.mozilla.com/D7827
devtools/client/shared/components/menu/MenuButton.js
--- a/devtools/client/shared/components/menu/MenuButton.js
+++ b/devtools/client/shared/components/menu/MenuButton.js
@@ -65,25 +65,27 @@ class MenuButton extends PureComponent {
     super(props);
 
     this.showMenu = this.showMenu.bind(this);
     this.hideMenu = this.hideMenu.bind(this);
     this.toggleMenu = this.toggleMenu.bind(this);
     this.onHidden = this.onHidden.bind(this);
     this.onClick = this.onClick.bind(this);
     this.onKeyDown = this.onKeyDown.bind(this);
+    this.onTouchStart = this.onTouchStart.bind(this);
 
     this.buttonRef = createRef();
 
     this.state = {
       expanded: false,
       // In tests, initialize the menu immediately.
       isMenuInitialized: flags.testing || false,
       win: props.doc.defaultView.top,
     };
+    this.ignoreNextClick = false;
 
     this.initializeTooltip();
   }
 
   componentDidMount() {
     if (!this.state.isMenuInitialized) {
       // Initialize the menu when the button is focused or moused over.
       for (const event of ["focus", "mousemove"]) {
@@ -191,16 +193,54 @@ class MenuButton extends PureComponent {
     }
 
     this.tooltip.updateContainerBounds(this.buttonRef.current, {
       position: this.props.menuPosition,
       y: this.props.menuOffset,
     });
   }
 
+  // When we are closing the menu we will get a 'hidden' event before we get
+  // a 'click' event. We want to re-enable the pointer-events: auto setting we
+  // use on the button while the menu is visible, but we don't want to do it
+  // until after the subsequent click event since otherwise we will end up
+  // re-opening the menu.
+  //
+  // For mouse events, we achieve this by using setTimeout(..., 0) to schedule
+  // a separate task to run after the click event, but in the case of touch
+  // events the event order differs and the setTimeout callback will run before
+  // the click event.
+  //
+  // In order to prevent that we detect touch events and set a flag to ignore
+  // the next click event. However, we need to differentiate between touch drag
+  // events and long press events (which don't generate a 'click') and "taps"
+  // (which do). We do that by looking for a 'touchmove' event and clearing the
+  // flag if we get one.
+  onTouchStart(evt) {
+    const touchend = () => {
+      const anchorRect = this.buttonRef.current.getClientRects()[0];
+      const { clientX, clientY } = evt.changedTouches[0];
+      // We need to check that the click is inside the bounds since when the
+      // menu is being closed the button will currently have
+      // pointer-events: none (and if we don't check the bounds we will end up
+      // ignoring unrelated clicks).
+      if (anchorRect.x <= clientX && clientX <= anchorRect.x + anchorRect.width &&
+          anchorRect.y <= clientY && clientY <= anchorRect.y + anchorRect.height) {
+        this.ignoreNextClick = true;
+      }
+    };
+
+    const touchmove = () => {
+      this.state.win.removeEventListener("touchend", touchend);
+    };
+
+    this.state.win.addEventListener("touchend", touchend, { once: true });
+    this.state.win.addEventListener("touchmove", touchmove, { once: true });
+  }
+
   onHidden() {
     this.setState({ expanded: false });
     // While the menu is open, if we click _anywhere_ outside the menu, it will
     // automatically close. This is performed by the XUL wrapper before we get
     // any chance to see any event. To avoid immediately re-opening the menu
     // when we process the subsequent click event on this button, we set
     // 'pointer-events: none' on the button while the menu is open.
     //
@@ -208,24 +248,34 @@ class MenuButton extends PureComponent {
     // the button works again) but we don't want to do it immediately since the
     // "popuphidden" event which triggers this callback might be dispatched
     // before the "click" event that we want to ignore.  As a result, we queue
     // up a task using setTimeout() to run after the "click" event.
     this.state.win.setTimeout(() => {
       if (this.buttonRef.current) {
         this.buttonRef.current.style.pointerEvents = "auto";
       }
+      this.state.win.removeEventListener("touchstart",
+                                         this.onTouchStart,
+                                         true);
     }, 0);
 
+    this.state.win.addEventListener("touchstart", this.onTouchStart, true);
+
     if (this.props.onCloseButton) {
       this.props.onCloseButton();
     }
   }
 
   async onClick(e) {
+    if (this.ignoreNextClick) {
+      this.ignoreNextClick = false;
+      return;
+    }
+
     if (e.target === this.buttonRef.current) {
       // On Mac, even after clicking the button it doesn't get focus.
       // Force focus to the button so that our keydown handlers get called.
       this.buttonRef.current.focus();
 
       if (this.props.onClick) {
         this.props.onClick(e);
       }