From 3d4b769dfca6c0cced1fb91be0a98cfbbc6c2d54 Mon Sep 17 00:00:00 2001 From: Brian Grinstead Date: Fri, 8 May 2015 22:42:05 -0700 Subject: [PATCH] Backed out changeset f407ab8f7129 (bug 1127423) for suspicion of causing a timeout on browser_mdn-docs-01.js --- browser/devtools/markupview/markup-view.js | 7 +- browser/devtools/shared/test/browser.ini | 1 + .../shared/test/browser_layoutHelpers.html | 1 + .../shared/test/browser_layoutHelpers.js | 69 +++++++++++-------- .../test/browser_layoutHelpers_iframe.html | 19 +++++ toolkit/devtools/LayoutHelpers.jsm | 42 +++++++++-- 6 files changed, 103 insertions(+), 36 deletions(-) create mode 100644 browser/devtools/shared/test/browser_layoutHelpers_iframe.html diff --git a/browser/devtools/markupview/markup-view.js b/browser/devtools/markupview/markup-view.js index b3ea3e9ad473..32e900de671a 100644 --- a/browser/devtools/markupview/markup-view.js +++ b/browser/devtools/markupview/markup-view.js @@ -150,7 +150,7 @@ MarkupView.prototype = { // Show markup-containers as hovered on toolbox "picker-node-hovered" event // which happens when the "pick" button is pressed this._onToolboxPickerHover = (event, nodeFront) => { - this.showNode(nodeFront).then(() => { + this.showNode(nodeFront, true).then(() => { this._showContainerAsHovered(nodeFront); }); }; @@ -436,7 +436,7 @@ MarkupView.prototype = { this._brieflyShowBoxModel(selection.nodeFront); } - this.showNode(selection.nodeFront).then(() => { + this.showNode(selection.nodeFront, true).then(() => { if (this._destroyer) { return promise.reject("markupview destroyed"); } @@ -836,7 +836,7 @@ MarkupView.prototype = { * Make sure the given node's parents are expanded and the * node is scrolled on to screen. */ - showNode: function(aNode, centered=true) { + showNode: function(aNode, centered) { let parent = aNode; this.importNode(aNode); @@ -852,6 +852,7 @@ MarkupView.prototype = { } return this._ensureVisible(aNode); }).then(() => { + // Why is this not working? this.layoutHelpers.scrollIntoViewIfNeeded(this.getContainer(aNode).editor.elt, centered); }, e => { // Only report this rejection as an error if the panel hasn't been diff --git a/browser/devtools/shared/test/browser.ini b/browser/devtools/shared/test/browser.ini index c8e93aaa8e12..db693f128e1a 100644 --- a/browser/devtools/shared/test/browser.ini +++ b/browser/devtools/shared/test/browser.ini @@ -4,6 +4,7 @@ subsuite = devtools support-files = browser_layoutHelpers.html browser_layoutHelpers-getBoxQuads.html + browser_layoutHelpers_iframe.html browser_templater_basic.html browser_toolbar_basic.html browser_toolbar_webconsole_errors_count.html diff --git a/browser/devtools/shared/test/browser_layoutHelpers.html b/browser/devtools/shared/test/browser_layoutHelpers.html index f50bfffdf96e..3b9a285b42b0 100644 --- a/browser/devtools/shared/test/browser_layoutHelpers.html +++ b/browser/devtools/shared/test/browser_layoutHelpers.html @@ -22,3 +22,4 @@
+ diff --git a/browser/devtools/shared/test/browser_layoutHelpers.js b/browser/devtools/shared/test/browser_layoutHelpers.js index 546c1e20b21e..3ee8665f1feb 100644 --- a/browser/devtools/shared/test/browser_layoutHelpers.js +++ b/browser/devtools/shared/test/browser_layoutHelpers.js @@ -2,85 +2,100 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ // Tests that scrollIntoViewIfNeeded works properly. -let {LayoutHelpers} = Cu.import("resource://gre/modules/devtools/LayoutHelpers.jsm", {}); +let imported = {}; +Components.utils.import("resource://gre/modules/devtools/LayoutHelpers.jsm", + imported); +registerCleanupFunction(function () { + imported = {}; +}); + +let LayoutHelpers = imported.LayoutHelpers; const TEST_URI = TEST_URI_ROOT + "browser_layoutHelpers.html"; -add_task(function*() { - let browser = yield promiseTab(TEST_URI); - let doc = browser.contentDocument; - yield runTest(doc.defaultView); - gBrowser.removeCurrentTab(); -}); +function test() { + addTab(TEST_URI, function(browser, tab) { + info("Starting browser_layoutHelpers.js"); + let doc = browser.contentDocument; + runTest(doc.defaultView, doc.getElementById('some')); + gBrowser.removeCurrentTab(); + finish(); + }); +} -function runTest(win) { +function runTest(win, some) { let lh = new LayoutHelpers(win); - let some = win.document.getElementById('some'); some.style.top = win.innerHeight + 'px'; some.style.left = win.innerWidth + 'px'; // The tests start with a black 2x2 pixels square below bottom right. // Do not resize the window during the tests. - let xPos = Math.floor(win.innerWidth / 2); - win.scroll(xPos, win.innerHeight + 2); // Above the viewport. + win.scroll(win.innerWidth / 2, win.innerHeight + 2); // Above the viewport. lh.scrollIntoViewIfNeeded(some); is(win.scrollY, Math.floor(win.innerHeight / 2) + 1, 'Element completely hidden above should appear centered.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, win.innerHeight + 1); // On the top edge. lh.scrollIntoViewIfNeeded(some); is(win.scrollY, win.innerHeight, 'Element partially visible above should appear above.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, 0); // Just below the viewport. lh.scrollIntoViewIfNeeded(some); is(win.scrollY, Math.floor(win.innerHeight / 2) + 1, 'Element completely hidden below should appear centered.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, 1); // On the bottom edge. lh.scrollIntoViewIfNeeded(some); is(win.scrollY, 2, 'Element partially visible below should appear below.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); + win.scroll(win.innerWidth / 2, win.innerHeight + 2); // Above the viewport. lh.scrollIntoViewIfNeeded(some, false); is(win.scrollY, win.innerHeight, 'Element completely hidden above should appear above ' + 'if parameter is false.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, win.innerHeight + 1); // On the top edge. lh.scrollIntoViewIfNeeded(some, false); is(win.scrollY, win.innerHeight, 'Element partially visible above should appear above ' + 'if parameter is false.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, 0); // Below the viewport. lh.scrollIntoViewIfNeeded(some, false); is(win.scrollY, 2, 'Element completely hidden below should appear below ' + 'if parameter is false.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); win.scroll(win.innerWidth / 2, 1); // On the bottom edge. lh.scrollIntoViewIfNeeded(some, false); is(win.scrollY, 2, 'Element partially visible below should appear below ' + 'if parameter is false.'); - is(win.scrollX, xPos, - 'scrollX position has not changed.'); + + // The case of iframes. + win.scroll(0, 0); + + let frame = win.document.getElementById('frame'); + let fwin = frame.contentWindow; + + frame.style.top = win.innerHeight + 'px'; + frame.style.left = win.innerWidth + 'px'; + + fwin.addEventListener('load', function frameLoad() { + let some = fwin.document.getElementById('some'); + lh.scrollIntoViewIfNeeded(some); + is(win.scrollX, Math.floor(win.innerWidth / 2) + 20, + 'Scrolling from an iframe should center the iframe vertically.'); + is(win.scrollY, Math.floor(win.innerHeight / 2) + 20, + 'Scrolling from an iframe should center the iframe horizontally.'); + is(fwin.scrollX, Math.floor(fwin.innerWidth / 2) + 1, + 'Scrolling from an iframe should center the element vertically.'); + is(fwin.scrollY, Math.floor(fwin.innerHeight / 2) + 1, + 'Scrolling from an iframe should center the element horizontally.'); + }, false); } diff --git a/browser/devtools/shared/test/browser_layoutHelpers_iframe.html b/browser/devtools/shared/test/browser_layoutHelpers_iframe.html new file mode 100644 index 000000000000..66ef5b293703 --- /dev/null +++ b/browser/devtools/shared/test/browser_layoutHelpers_iframe.html @@ -0,0 +1,19 @@ + + + Layout Helpers + + + +
+ diff --git a/toolkit/devtools/LayoutHelpers.jsm b/toolkit/devtools/LayoutHelpers.jsm index d3963fec0c47..7b82e844da1b 100644 --- a/toolkit/devtools/LayoutHelpers.jsm +++ b/toolkit/devtools/LayoutHelpers.jsm @@ -216,35 +216,38 @@ LayoutHelpers.prototype = { }, /** - * Scroll the document so that the element "elem" appears vertically in - * the viewport. + * Scroll the document so that the element "elem" appears in the viewport. * * @param {DOMNode} elem * The element that needs to appear in the viewport. * @param {Boolean} centered * true if you want it centered, false if you want it to appear on the - * top of the viewport. True by default, and that is usually what + * top of the viewport. It is true by default, and that is usually what * you want. */ scrollIntoViewIfNeeded: function(elem, centered) { // We want to default to centering the element in the page, // so as to keep the context of the element. - centered = centered === undefined ? true: !!centered; + centered = centered === undefined? true: !!centered; let win = elem.ownerDocument.defaultView; let clientRect = elem.getBoundingClientRect(); - // The following are always from the {top, bottom} + // The following are always from the {top, bottom, left, right} // of the viewport, to the {top, …} of the box. // Think of them as geometrical vectors, it helps. // The origin is at the top left. let topToBottom = clientRect.bottom; let bottomToTop = clientRect.top - win.innerHeight; - let yAllowed = true; // We allow one translation on the y axis. + let leftToRight = clientRect.right; + let rightToLeft = clientRect.left - win.innerWidth; + let xAllowed = true; // We allow one translation on the x axis, + let yAllowed = true; // and one on the y axis. // Whatever `centered` is, the behavior is the same if the box is // (even partially) visible. + if ((topToBottom > 0 || !centered) && topToBottom <= elem.offsetHeight) { win.scrollBy(0, topToBottom - elem.offsetHeight); yAllowed = false; @@ -254,14 +257,41 @@ LayoutHelpers.prototype = { yAllowed = false; } + if ((leftToRight > 0 || !centered) && leftToRight <= elem.offsetWidth) { + if (xAllowed) { + win.scrollBy(leftToRight - elem.offsetWidth, 0); + xAllowed = false; + } + } else + if ((rightToLeft < 0 || !centered) && rightToLeft >= -elem.offsetWidth) { + if (xAllowed) { + win.scrollBy(rightToLeft + elem.offsetWidth, 0); + xAllowed = false; + } + } + // If we want it centered, and the box is completely hidden, // then we center it explicitly. + if (centered) { + if (yAllowed && (topToBottom <= 0 || bottomToTop >= 0)) { win.scroll(win.scrollX, win.scrollY + clientRect.top - (win.innerHeight - elem.offsetHeight) / 2); } + + if (xAllowed && (leftToRight <= 0 || rightToLeft <= 0)) { + win.scroll(win.scrollX + clientRect.left + - (win.innerWidth - elem.offsetWidth) / 2, + win.scrollY); + } + } + + if (!this.isTopLevelWindow(win)) { + // We are inside an iframe. + let frameElement = this.getFrameElement(win); + this.scrollIntoViewIfNeeded(frameElement, centered); } },