Bug 1488991: Update remote browser position before showing context menu. r=pbro,smaug
authorDaisuke Akatsuka <dakatsuka@mozilla.com>
Fri, 15 Mar 2019 00:09:20 +0000
changeset 464110 9d33ee85f2d4e5f45d6c795c2be3c27ec31f2bae
parent 464109 f28acbccbc62a92d3116b30f35afcd7203ff07d7
child 464111 468eed96e879402c4332a9cc3aff1c28d8eb50e0
push id112438
push useropoprus@mozilla.com
push dateFri, 15 Mar 2019 09:46:56 +0000
treeherdermozilla-inbound@9daec44d681e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro, smaug
bugs1488991
milestone67.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 1488991: Update remote browser position before showing context menu. r=pbro,smaug The position of remote browser was not updated by resizing the window and changing the align of viewport etc, although will be updated when the window moves, the frame reflows and so on. Thus, in this patch, update the position of remote browser before showing context menu so as to locates at proper position. I investigated though, when reflow and moving happens, the position is updated by TabParent::UpdateDimensions()[1]. This patch as well is taking an approach which update the position explicitly by TabParent::UpdateDimensions() before showing context menu. [1] https://searchfox.org/mozilla-central/source/dom/ipc/TabParent.cpp#729 Differential Revision: https://phabricator.services.mozilla.com/D23470
devtools/client/responsive.html/components/App.js
devtools/client/responsive.html/components/Browser.js
--- a/devtools/client/responsive.html/components/App.js
+++ b/devtools/client/responsive.html/components/App.js
@@ -54,16 +54,17 @@ class App extends PureComponent {
       viewports: PropTypes.arrayOf(PropTypes.shape(Types.viewport)).isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.onAddCustomDevice = this.onAddCustomDevice.bind(this);
+    this.onBrowserContextMenu = this.onBrowserContextMenu.bind(this);
     this.onBrowserMounted = this.onBrowserMounted.bind(this);
     this.onChangeDevice = this.onChangeDevice.bind(this);
     this.onChangeNetworkThrottling = this.onChangeNetworkThrottling.bind(this);
     this.onChangePixelRatio = this.onChangePixelRatio.bind(this);
     this.onChangeTouchSimulation = this.onChangeTouchSimulation.bind(this);
     this.onChangeUserAgent = this.onChangeUserAgent.bind(this);
     this.onContentResize = this.onContentResize.bind(this);
     this.onDeviceListUpdate = this.onDeviceListUpdate.bind(this);
@@ -77,21 +78,34 @@ class App extends PureComponent {
     this.onToggleReloadOnTouchSimulation =
       this.onToggleReloadOnTouchSimulation.bind(this);
     this.onToggleReloadOnUserAgent = this.onToggleReloadOnUserAgent.bind(this);
     this.onToggleUserAgentInput = this.onToggleUserAgentInput.bind(this);
     this.onUpdateDeviceDisplayed = this.onUpdateDeviceDisplayed.bind(this);
     this.onUpdateDeviceModal = this.onUpdateDeviceModal.bind(this);
   }
 
+  componentWillUnmount() {
+    this.browser.removeEventListener("contextmenu", this.onContextMenu);
+    this.browser = null;
+  }
+
   onAddCustomDevice(device) {
     this.props.dispatch(addCustomDevice(device));
   }
 
-  onBrowserMounted() {
+  onBrowserContextMenu() {
+    // Update the position of remote browser so that makes the context menu to show at
+    // proper position before showing.
+    this.browser.frameLoader.requestUpdatePosition();
+  }
+
+  onBrowserMounted(browser) {
+    this.browser = browser;
+    this.browser.addEventListener("contextmenu", this.onBrowserContextMenu);
     window.postMessage({ type: "browser-mounted" }, "*");
   }
 
   onChangeDevice(id, device, deviceType) {
     // TODO: Bug 1332754: Move messaging and logic into the action creator so that the
     // message is sent from the action creator and device property changes are sent from
     // there instead of this function.
     window.postMessage({
--- a/devtools/client/responsive.html/components/Browser.js
+++ b/devtools/client/responsive.html/components/Browser.js
@@ -67,17 +67,17 @@ class Browser extends PureComponent {
     // script now.
     if (!this.props.swapAfterMount) {
       await this.startFrameScript();
     }
 
     // Notify manager.js that this browser has mounted, so that it can trigger
     // a swap if needed and continue with the rest of its startup.
     await this.browserShown;
-    this.props.onBrowserMounted();
+    this.props.onBrowserMounted(this.browser);
 
     // If we are swapping browsers after mount, wait for the swap to complete
     // and start the frame script after that.
     if (this.props.swapAfterMount) {
       await message.wait(window, "start-frame-script");
       await this.startFrameScript();
       message.post(window, "start-frame-script:done");
     }