Skip to content

Commit

Permalink
Improve comments and error message on scrolling_test#testScrollingAcc…
Browse files Browse the repository at this point in the history
…ountsForScrollbarWidths.

Partially reverts eda96a0. That commit implies that the test was scrolling exactly 26px, which is incorrect behavior; lowering the threshold to 25px defeats the purpose of the test.

Signed-off-by: Luke Inman-Semerau <luke.semerau@gmail.com>
  • Loading branch information
juangj authored and lukeis committed Aug 17, 2015
1 parent ca1e60e commit 26b6d70
Showing 1 changed file with 18 additions and 11 deletions.
29 changes: 18 additions & 11 deletions javascript/atoms/test/scrolling_test.html
Original file line number Diff line number Diff line change
Expand Up @@ -209,23 +209,30 @@
bot.action.scrollIntoView(elem, new goog.math.Coordinate(75, 75));
var rectAfter = bot.dom.getClientRect(elem);

// Seems like chrome on MacOs is stealing a pixel
var expected = goog.labs.userAgent.browser.isChrome() && goog.labs.userAgent.platform.isMacintosh() ?
25 : 26;

// If the scrollbar isn't taken into account, the minimum scrolling
// needed would be 26 pixels, to bring the full pixel into view.
var horizontalScroll = rectBefore.left - rectAfter.left;
var verticalScroll = rectBefore.top - rectAfter.top;

// In the absence of a scrollbar, the minimum scrolling needed to bring
// the full pixel into view would be 26 pixels. There is a scrollbar,
// however; scrollIntoView should scroll 26px plus the scrollbar
// width/height.

// This should fail on mobile and tablet since the scrollbars aren't shown.
// This should also fail on IE6 since scrolling only happens to
// exactly the scrollbar width, which is 17 by default on Windows XP.
expectedFailures.expectFailureFor(
goog.labs.userAgent.device.isMobile() || goog.labs.userAgent.device.isTablet() ||
(goog.userAgent.IE && !goog.userAgent.isVersion(7)));
(goog.userAgent.IE && !goog.userAgent.isVersion(7)));

expectedFailures.run(function() {
assertTrue('horizontal scroll did not account for scrollbar' + (rectBefore.left - rectAfter.left),
rectBefore.left - rectAfter.left > expected);
assertTrue('vertical scroll did not account for scrollbar',
rectBefore.top - rectAfter.top > 26);
assertTrue('Horizontal scroll did not account for scrollbar. ' +
'It scrolled ' + horizontalScroll + 'px, but should have ' +
'scrolled at least 26px plus the scrollbar width.',
horizontalScroll > 26);
assertTrue('Vertical scroll did not account for scrollbar. ' +
'It scrolled ' + verticalScroll + 'px, but should have ' +
'scrolled at least 26px plus the scrollbar height.',
verticalScroll > 26);
});
}
</script>
Expand Down

0 comments on commit 26b6d70

Please sign in to comment.