Bug 1382341 - removed `this._remove()` from `onWindowReady`; r=pbro
authorZER0 <zer0.kaos@gmail.com>
Tue, 05 Sep 2017 15:23:18 +0200
changeset 428475 dd3069bc00a3ba22f68f3881939a4b7c466bde41
parent 428474 b4f0f129495937add3d9b95e5af2dffa5a943ed5
child 428476 5a1e1ee5aa23c4d7a226a907137357efc6c9d0b5
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1382341, 1341756, 1396690
milestone57.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 1382341 - removed `this._remove()` from `onWindowReady`; r=pbro This basically revert the changes made in bug 1341756; but it doesn't seems to bring the same issue as before since `pagehide` event is emitted properly when entering in RDM. Added unit test to catch future regression. The removal of duplicated hidden markup in anonymous content will be handled by Bug 1396690. MozReview-Commit-ID: 92qC9CHYL1a
devtools/client/inspector/test/browser.ini
devtools/client/inspector/test/browser_inspector_highlighter-06.js
devtools/client/inspector/test/doc_inspector_highlighter_scroll.html
devtools/server/actors/highlighters/utils/markup.js
--- a/devtools/client/inspector/test/browser.ini
+++ b/devtools/client/inspector/test/browser.ini
@@ -17,16 +17,17 @@ support-files =
   doc_inspector_highlighter-geometry_02.html
   doc_inspector_highlighter_cssshapes.html
   doc_inspector_highlighter_csstransform.html
   doc_inspector_highlighter_dom.html
   doc_inspector_highlighter_inline.html
   doc_inspector_highlighter.html
   doc_inspector_highlighter_rect.html
   doc_inspector_highlighter_rect_iframe.html
+  doc_inspector_highlighter_scroll.html
   doc_inspector_highlighter_xbl.xul
   doc_inspector_infobar_01.html
   doc_inspector_infobar_02.html
   doc_inspector_infobar_03.html
   doc_inspector_infobar_textnode.html
   doc_inspector_long-divs.html
   doc_inspector_menu.html
   doc_inspector_outerhtml.html
@@ -65,16 +66,17 @@ skip-if = os == "mac" # Full keyboard na
 [browser_inspector_destroy-before-ready.js]
 [browser_inspector_expand-collapse.js]
 [browser_inspector_gcli-inspect-command.js]
 [browser_inspector_highlighter-01.js]
 [browser_inspector_highlighter-02.js]
 [browser_inspector_highlighter-03.js]
 [browser_inspector_highlighter-04.js]
 [browser_inspector_highlighter-05.js]
+[browser_inspector_highlighter-06.js]
 [browser_inspector_highlighter-by-type.js]
 [browser_inspector_highlighter-cancel.js]
 [browser_inspector_highlighter-comments.js]
 [browser_inspector_highlighter-cssgrid_01.js]
 [browser_inspector_highlighter-cssgrid_02.js]
 [browser_inspector_highlighter-cssshape_01.js]
 [browser_inspector_highlighter-cssshape_02.js]
 [browser_inspector_highlighter-cssshape_03.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-06.js
@@ -0,0 +1,31 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+// Test that the browser remembers the previous scroll position after reload, even with
+// Inspector opened - Bug 1382341.
+// NOTE: Using a real file instead data: URL since the bug won't happen on data: URL
+const TEST_URI = URL_ROOT + "doc_inspector_highlighter_scroll.html";
+
+add_task(function* () {
+  let { inspector, testActor } = yield openInspectorForURL(TEST_URI);
+
+  yield testActor.scrollIntoView("a");
+  yield selectAndHighlightNode("a", inspector);
+
+  let markupLoaded = inspector.once("markuploaded");
+
+  let y = yield testActor.eval("window.pageYOffset");
+  isnot(y, 0, "window scrolled vertically.");
+
+  info("Reloading page.");
+  yield testActor.reload();
+
+  info("Waiting for markupview to load after reload.");
+  yield markupLoaded;
+
+  let newY = yield testActor.eval("window.pageYOffset");
+  is(y, newY, "window remember the previous scroll position.");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/doc_inspector_highlighter_scroll.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <style>
+      p {
+        height: 200vh;
+      }
+    </style>
+  </head>
+  <body>
+    <p>Bug 1382341 - test page reload and scroll position</p>
+    <a>An element anchor, used to scroll the page</a>
+  </body>
+</html>
--- a/devtools/server/actors/highlighters/utils/markup.js
+++ b/devtools/server/actors/highlighters/utils/markup.js
@@ -303,17 +303,16 @@ CanvasFrameAnonymousContentHelper.protot
    * The "window-ready" event can be triggered when:
    *   - a new window is created
    *   - a window is unfrozen from bfcache
    *   - when first attaching to a page
    *   - when swapping frame loaders (moving tabs, toggling RDM)
    */
   _onWindowReady(e, {isTopLevel}) {
     if (isTopLevel) {
-      this._remove();
       this._removeAllListeners();
       this.elements.clear();
       this._insert();
       this.anonymousContentDocument = this.highlighterEnv.document;
     }
   },
 
   getComputedStylePropertyValue(id, property) {