Backed out changeset 03b1762fb073 (bug 1548369) for devtools failures at devtools/client/debugger/test/mochitest/browser_dbg-inspector-integration.js on a CLOSED TREE
authorCoroiu Cristina <ccoroiu@mozilla.com>
Wed, 15 May 2019 22:26:56 +0300
changeset 532817 9dd4eba320e64aea32ba5e62b96c14c93768427f
parent 532816 f3fdb2fd9fa126bcfeb564bfe550ef781cc81756
child 532818 be2fb503011adec5a8d387f2bc531e13fab0fd8e
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1548369
milestone68.0a1
backs out03b1762fb0734dc1f58ce5453a8aef8770745538
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
Backed out changeset 03b1762fb073 (bug 1548369) for devtools failures at devtools/client/debugger/test/mochitest/browser_dbg-inspector-integration.js on a CLOSED TREE
devtools/client/debugger/src/actions/preview.js
devtools/client/debugger/src/components/Editor/Preview/Popup.js
devtools/client/debugger/src/components/Editor/Preview/index.js
devtools/client/debugger/src/components/shared/Popover.js
devtools/client/debugger/src/reducers/ast.js
--- a/devtools/client/debugger/src/actions/preview.js
+++ b/devtools/client/debugger/src/actions/preview.js
@@ -65,37 +65,32 @@ export function updatePreview(
     }
 
     const { expression, location } = match;
 
     if (isConsole(expression)) {
       return;
     }
 
-    dispatch(setPreview(cx, expression, location, tokenPos, cursorPos, target));
+    dispatch(setPreview(cx, expression, location, tokenPos, cursorPos));
   };
 }
 
 export function setPreview(
   cx: Context,
   expression: string,
   location: AstLocation,
   tokenPos: Position,
-  cursorPos: ClientRect,
-  target: HTMLElement
+  cursorPos: ClientRect
 ) {
   return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
     await dispatch({
       type: "SET_PREVIEW",
       cx,
       [PROMISE]: (async function() {
-        if (getPreview(getState())) {
-          dispatch(clearPreview(cx));
-        }
-
         const source = getSelectedSource(getState());
         if (!source) {
           return;
         }
 
         const thread = getCurrentThread(getState());
         const selectedFrame = getSelectedFrame(getState(), thread);
 
@@ -131,32 +126,24 @@ export function setPreview(
 
         const root = {
           name: expression,
           path: expression,
           contents: { value: result }
         };
         const properties = await client.loadObjectProperties(root);
 
-        // The first time a popup is rendered, the mouse should be hovered
-        // on the token. If it happens to be hovered on whitespace, it should
-        // not render anything
-        if (!target.matches(":hover")) {
-          return;
-        }
-
         return {
           expression,
           result,
           properties,
           root,
           location,
           tokenPos,
-          cursorPos,
-          target
+          cursorPos
         };
       })()
     });
   };
 }
 
 export function clearPreview(cx: Context) {
   return ({ dispatch, getState, client }: ThunkArgs) => {
--- a/devtools/client/debugger/src/components/Editor/Preview/Popup.js
+++ b/devtools/client/debugger/src/components/Editor/Preview/Popup.js
@@ -16,90 +16,92 @@ const {
 
 const { ObjectInspector, utils } = objectInspector;
 
 const {
   node: { nodeIsPrimitive, nodeIsFunction, nodeIsObject }
 } = utils;
 
 import actions from "../../../actions";
-import { getThreadContext, getPreview } from "../../../selectors";
+import { getThreadContext } from "../../../selectors";
 import Popover from "../../shared/Popover";
 import PreviewFunction from "../../shared/PreviewFunction";
 
 import { createObjectClient } from "../../../client/firefox";
 
 import "./Popup.css";
 
 import type { Coords } from "../../shared/Popover";
 import type { ThreadContext } from "../../../types";
 import type { PreviewValue } from "../../../reducers/types";
 
 type Props = {
   cx: ThreadContext,
   preview: PreviewValue,
+  onClose: () => void,
   editor: any,
   editorRef: ?HTMLDivElement,
   addExpression: typeof actions.addExpression,
   selectSourceURL: typeof actions.selectSourceURL,
   openLink: typeof actions.openLink,
   openElementInInspector: typeof actions.openElementInInspectorCommand,
   highlightDomElement: typeof actions.highlightDomElement,
-  unHighlightDomElement: typeof actions.unHighlightDomElement,
-  clearPreview: typeof actions.clearPreview
+  unHighlightDomElement: typeof actions.unHighlightDomElement
 };
 
 type State = {
   top: number
 };
 
+function inPreview(event) {
+  const relatedTarget: Element = (event.relatedTarget: any);
+
+  if (
+    !relatedTarget ||
+    (relatedTarget.classList &&
+      relatedTarget.classList.contains("preview-expression"))
+  ) {
+    return true;
+  }
+
+  // $FlowIgnore
+  const inPreviewSelection = document
+    .elementsFromPoint(event.clientX, event.clientY)
+    .some(el => el.classList.contains("preview-selection"));
+
+  return inPreviewSelection;
+}
+
 export class Popup extends Component<Props, State> {
   marker: any;
   pos: any;
-  popup: ?HTMLDivElement;
-  timerId: ?IntervalID;
 
   constructor(props: Props) {
     super(props);
     this.state = {
       top: 0
     };
   }
 
-  componentDidMount() {
-    this.startTimer();
-  }
-
-  componentWillUnmount() {
-    if (this.timerId) {
-      clearInterval(this.timerId);
-    }
-  }
+  onMouseLeave = (e: SyntheticMouseEvent<HTMLDivElement>) => {
+    const relatedTarget: Element = (e.relatedTarget: any);
 
-  startTimer() {
-    this.timerId = setInterval(this.onInterval, 300);
-  }
-
-  onInterval = () => {
-    const { preview, clearPreview, cx } = this.props;
-
-    // Don't clear the current preview if mouse is hovered on
-    // the current preview's element (target) or the popup element
-    const currentTarget = preview.target;
-    if (
-      currentTarget.matches(":hover") ||
-      (this.popup && this.popup.matches(":hover"))
-    ) {
-      return;
+    if (!relatedTarget) {
+      return this.props.onClose();
     }
 
-    // Clear the interval and the preview if it is not hovered
-    // on the current preview's element or the popup element
-    clearInterval(this.timerId);
-    return clearPreview(cx);
+    if (!inPreview(e)) {
+      this.props.onClose();
+    }
+  };
+
+  onKeyDown = (e: KeyboardEvent) => {
+    if (e.key === "Escape") {
+      this.props.onClose();
+    }
   };
 
   calculateMaxHeight = () => {
     const { editorRef } = this.props;
     if (!editorRef) {
       return "auto";
     }
     return editorRef.getBoundingClientRect().height - this.state.top;
@@ -110,17 +112,16 @@ export class Popup extends Component<Pro
       cx,
       selectSourceURL,
       preview: { result }
     } = this.props;
 
     return (
       <div
         className="preview-popup"
-        ref={a => (this.popup = a)}
         onClick={() =>
           selectSourceURL(cx, result.location.url, {
             line: result.location.line
           })
         }
       >
         <PreviewFunction func={result} />
       </div>
@@ -135,17 +136,16 @@ export class Popup extends Component<Pro
       highlightDomElement,
       unHighlightDomElement
     } = this.props;
 
     return (
       <div
         className="preview-popup"
         style={{ maxHeight: this.calculateMaxHeight() }}
-        ref={a => (this.popup = a)}
       >
         <ObjectInspector
           roots={properties}
           autoExpandDepth={0}
           disableWrap={true}
           focusable={false}
           openLink={openLink}
           createObjectClient={grip => createObjectClient(grip)}
@@ -159,17 +159,17 @@ export class Popup extends Component<Pro
   }
 
   renderSimplePreview() {
     const {
       openLink,
       preview: { result }
     } = this.props;
     return (
-      <div className="preview-popup" ref={a => (this.popup = a)}>
+      <div className="preview-popup">
         {Rep({
           object: result,
           mode: MODE.LONG,
           openLink
         })}
       </div>
     );
   }
@@ -218,47 +218,46 @@ export class Popup extends Component<Pro
 
     if (typeof result == "undefined" || result.optimizedOut) {
       return null;
     }
 
     return (
       <Popover
         targetPosition={cursorPos}
+        onMouseLeave={this.onMouseLeave}
+        onKeyDown={this.onKeyDown}
         type={type}
         onPopoverCoords={this.onPopoverCoords}
         editorRef={editorRef}
       >
         {this.renderPreview()}
       </Popover>
     );
   }
 }
 
 const mapStateToProps = state => ({
-  cx: getThreadContext(state),
-  preview: getPreview(state)
+  cx: getThreadContext(state)
 });
 
 const {
   addExpression,
   selectSourceURL,
   openLink,
   openElementInInspectorCommand,
   highlightDomElement,
-  unHighlightDomElement,
-  clearPreview
+  unHighlightDomElement
 } = actions;
 
 const mapDispatchToProps = {
   addExpression,
   selectSourceURL,
   openLink,
   openElementInInspector: openElementInInspectorCommand,
   highlightDomElement,
-  unHighlightDomElement,
-  clearPreview
+  unHighlightDomElement
 };
 
 export default connect(
   mapStateToProps,
   mapDispatchToProps
 )(Popup);
--- a/devtools/client/debugger/src/components/Editor/Preview/index.js
+++ b/devtools/client/debugger/src/components/Editor/Preview/index.js
@@ -25,16 +25,29 @@ type Props = {
   addExpression: typeof actions.addExpression,
   updatePreview: typeof actions.updatePreview
 };
 
 type State = {
   selecting: boolean
 };
 
+function inPopup(e) {
+  const { relatedTarget } = e;
+
+  if (!relatedTarget) {
+    return true;
+  }
+
+  const pop =
+    relatedTarget.closest(".tooltip") || relatedTarget.closest(".popover");
+
+  return pop;
+}
+
 function getElementFromPos(pos: DOMRect) {
   // We need to use element*s*AtPoint because the tooltip overlays
   // the token and thus an undesirable element may be returned
   const elementsAtPoint = [
     // $FlowIgnore
     ...document.elementsFromPoint(pos.x + pos.width / 2, pos.y + pos.height / 2)
   ];
 
@@ -51,67 +64,60 @@ class Preview extends PureComponent<Prop
   componentDidMount() {
     this.updateListeners();
   }
 
   componentWillUnmount() {
     const { codeMirror } = this.props.editor;
     const codeMirrorWrapper = codeMirror.getWrapperElement();
 
+    codeMirror.off("scroll", this.onScroll);
     codeMirror.off("tokenenter", this.onTokenEnter);
-    codeMirror.off("scroll", this.onScroll);
+    codeMirror.off("tokenleave", this.onTokenLeave);
     codeMirrorWrapper.removeEventListener("mouseup", this.onMouseUp);
     codeMirrorWrapper.removeEventListener("mousedown", this.onMouseDown);
   }
 
   componentDidUpdate(prevProps) {
     this.updateHighlight(prevProps);
   }
 
   updateListeners(prevProps: ?Props) {
     const { codeMirror } = this.props.editor;
     const codeMirrorWrapper = codeMirror.getWrapperElement();
+    codeMirror.on("scroll", this.onScroll);
     codeMirror.on("tokenenter", this.onTokenEnter);
-    codeMirror.on("scroll", this.onScroll);
+    codeMirror.on("tokenleave", this.onTokenLeave);
     codeMirrorWrapper.addEventListener("mouseup", this.onMouseUp);
     codeMirrorWrapper.addEventListener("mousedown", this.onMouseDown);
   }
 
   updateHighlight(prevProps) {
     const { preview } = this.props;
 
-    if (preview && !preview.updating && preview.target.matches(":hover")) {
+    if (preview && !preview.updating) {
       const target = getElementFromPos(preview.cursorPos);
       target && target.classList.add("preview-selection");
     }
 
-    if (
-      prevProps.preview &&
-      !prevProps.preview.updating &&
-      prevProps.preview !== preview
-    ) {
+    if (prevProps.preview && !prevProps.preview.updating) {
       const target = getElementFromPos(prevProps.preview.cursorPos);
       target && target.classList.remove("preview-selection");
     }
   }
 
   onTokenEnter = ({ target, tokenPos }) => {
-    const { cx, editor, updatePreview, preview } = this.props;
-
+    const { cx, updatePreview, editor } = this.props;
     if (cx.isPaused) {
-      if (preview && target === preview.target) {
-        return;
-      }
-
       updatePreview(cx, target, tokenPos, editor.codeMirror);
     }
   };
 
-  onScroll = () => {
-    if (this.props.cx.isPaused) {
+  onTokenLeave = e => {
+    if (this.props.cx.isPaused && !inPopup(e)) {
       this.props.clearPreview(this.props.cx);
     }
   };
 
   onMouseUp = () => {
     if (this.props.cx.isPaused) {
       this.setState({ selecting: false });
       return true;
@@ -120,27 +126,40 @@ class Preview extends PureComponent<Prop
 
   onMouseDown = () => {
     if (this.props.cx.isPaused) {
       this.setState({ selecting: true });
       return true;
     }
   };
 
+  onScroll = () => {
+    if (this.props.cx.isPaused) {
+      this.props.clearPreview(this.props.cx);
+    }
+  };
+
+  onClose = e => {
+    if (this.props.cx.isPaused) {
+      this.props.clearPreview(this.props.cx);
+    }
+  };
+
   render() {
     const { preview } = this.props;
     if (!preview || preview.updating || this.state.selecting) {
       return null;
     }
 
     return (
       <Popup
         preview={preview}
         editor={this.props.editor}
         editorRef={this.props.editorRef}
+        onClose={this.onClose}
       />
     );
   }
 }
 
 const mapStateToProps = state => ({
   cx: getThreadContext(state),
   preview: getPreview(state)
--- a/devtools/client/debugger/src/components/shared/Popover.js
+++ b/devtools/client/debugger/src/components/shared/Popover.js
@@ -9,16 +9,18 @@ import BracketArrow from "./BracketArrow
 
 import "./Popover.css";
 
 type Props = {
   editorRef: ?HTMLDivElement,
   targetPosition: Object,
   children: ?React$Element<any>,
   onPopoverCoords: Function,
+  onMouseLeave: (e: SyntheticMouseEvent<HTMLDivElement>) => void,
+  onKeyDown: (e: KeyboardEvent) => void,
   type?: "popover" | "tooltip"
 };
 
 type Orientation = "up" | "down" | "right";
 type TargetMid = {
   x: number,
   y: number
 };
@@ -39,17 +41,19 @@ class Popover extends Component<Props, S
       left: 0,
       top: 0,
       orientation: "down",
       targetMid: { x: 0, y: 0 }
     }
   };
 
   static defaultProps = {
+    onMouseLeave: () => {},
     onPopoverCoords: () => {},
+    onKeyDown: () => {},
     type: "popover"
   };
 
   componentDidMount() {
     const { type } = this.props;
     const coords =
       type == "popover" ? this.getPopoverCoords() : this.getTooltipCoords();
 
@@ -220,37 +224,43 @@ class Popover extends Component<Props, S
       Object.assign(arrowProps, { orientation: "left", top, left: -9 });
     }
 
     return <BracketArrow {...arrowProps} />;
   }
 
   renderPopover() {
     const { top, left, orientation, targetMid } = this.state.coords;
+    const { onMouseLeave, onKeyDown } = this.props;
     const arrow = this.getPopoverArrow(orientation, targetMid.x, targetMid.y);
 
     return (
       <div
         className={classNames("popover", `orientation-${orientation}`, {
           up: orientation === "up"
         })}
+        onMouseLeave={onMouseLeave}
+        onKeyDown={onKeyDown}
         style={{ top, left }}
         ref={c => (this.$popover = c)}
       >
         {arrow}
         {this.getChildren()}
       </div>
     );
   }
 
   renderTooltip() {
     const { top, left } = this.state.coords;
+    const { onMouseLeave, onKeyDown } = this.props;
     return (
       <div
         className="tooltip"
+        onMouseLeave={onMouseLeave}
+        onKeyDown={onKeyDown}
         style={{ top, left }}
         ref={c => (this.$tooltip = c)}
       >
         {this.getChildren()}
       </div>
     );
   }
 
--- a/devtools/client/debugger/src/reducers/ast.js
+++ b/devtools/client/debugger/src/reducers/ast.js
@@ -34,18 +34,17 @@ export type Preview = {| updating: true 
 export type PreviewValue = {|
   expression: string,
   result: Grip,
   root: Node,
   properties: GripProperties,
   location: AstLocation,
   cursorPos: any,
   tokenPos: AstLocation,
-  updating: false,
-  target: HTMLDivElement
+  updating: false
 |};
 
 export type ASTState = {
   +symbols: SymbolsMap,
   +outOfScopeLocations: ?Array<AstLocation>,
   +inScopeLines: ?Array<number>,
   +preview: Preview
 };