Bug 1127423 - Don't scroll horizontally when selecting an element in markup view;r=jryans

This commit is contained in:
Brian Grinstead 2015-05-08 20:39:23 -07:00
parent e22382b934
commit 2b4d186a1a
6 changed files with 35 additions and 103 deletions

View File

@ -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, true).then(() => {
this.showNode(nodeFront).then(() => {
this._showContainerAsHovered(nodeFront);
});
};
@ -436,7 +436,7 @@ MarkupView.prototype = {
this._brieflyShowBoxModel(selection.nodeFront);
}
this.showNode(selection.nodeFront, true).then(() => {
this.showNode(selection.nodeFront).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) {
showNode: function(aNode, centered=true) {
let parent = aNode;
this.importNode(aNode);
@ -852,7 +852,6 @@ 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

View File

@ -4,7 +4,6 @@ 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

View File

@ -22,4 +22,3 @@
</style>
<div id=some></div>
<iframe id=frame src='./browser_layoutHelpers_iframe.html'></iframe>

View File

@ -2,100 +2,84 @@
* 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";
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();
});
}
add_task(function*() {
let [host, win, doc] = yield createHost("bottom", TEST_URI);
runTest(win);
host.destroy();
});
function runTest(win, some) {
function runTest(win) {
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.
win.scroll(win.innerWidth / 2, win.innerHeight + 2); // Above the viewport.
let xPos = Math.floor(win.innerWidth / 2);
win.scroll(xPos, 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.');
// 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);
is(win.scrollX, xPos,
'scrollX position has not changed.');
}

View File

@ -1,19 +0,0 @@
<!doctype html>
<meta charset=utf-8>
<title> Layout Helpers </title>
<style>
html {
height: 300%;
width: 300%;
}
div#some {
position: absolute;
background: black;
width: 2px;
height: 2px;
}
</style>
<div id=some></div>

View File

@ -216,38 +216,35 @@ LayoutHelpers.prototype = {
},
/**
* Scroll the document so that the element "elem" appears in the viewport.
* Scroll the document so that the element "elem" appears vertically 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. It is true by default, and that is usually what
* top of the viewport. 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, left, right}
// The following are always from the {top, bottom}
// 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 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.
let yAllowed = true; // We allow one translation 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;
@ -257,41 +254,14 @@ 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);
}
},