Changes to scrollWidth/scrollHeight



All timestamps are based on your local time of:

Posted by: stak
Tags: mozilla
Posted on: 2012-06-21 09:33:22

I just landed bug 755971 on mozilla-inbound. Assuming it sticks, it changes the scrollWidth and scrollHeight properties on non-scrollable elements so that it doesn't include the border of the element. This means that for an element that looks like this:

<div style="display: table; overflow: auto; width: 100px; height: 100px; border: 5px solid black">foo</div>

the scrollWidth and scrollHeight change from 110 to 100.

This behaviour is poorly specified and not consistent across browsers, but this brings Firefox more in line with the definitions in the CSSOM-View specification. It also makes the behaviour consistent across scrollable and non-scrollable elements - in both cases it now returns the padding box size. The behaviour for scrollable elements is unchanged.

Although this change makes sense in general, the reason it came up is because some of our code tries to detect if elements are scrollable by checking if their scrollWidth (or scrollHeight) is larger than their clientWidth (or clientHeight). This check was returning true in cases where non-scrollable elements had a border. By the looks of some questions on stackoverflow, this seems to be a general problem people are encountering, so it made sense to fix.

Posted by avih at 2012-06-21 10:48:08
So, what IS the correct and fully-working method to detect if an object is scrollable? Surely there's some code within Gecko that does that, maybe it can be exposed? even as a mozilla-specific property?

Addons which replace scrolling (SmoothWheel, all in one gestures, Yet another smooth scroll, etc) deploy a custom built algorithm for that (which are slightly different than each other, and are never working exactly as the built in detection for scrollable element under the mouse pointer), maybe Mozilla can make it easier for addons developers?
[ Reply to this ]
Posted by stak at 2012-06-21 16:01:47
Currently, the best way to detect if an element is scrollable is still "element.scrollWidth > element.clientWidth || element.scrollHeight > element.clientHeight". With the above-mentioned change, this works a little bit better than it used. However, it can still give wrong answers in some cases (such as when there are rounding errors that cause scrollWidth to be bigger than clientWidth, but the un-rounded values are less than a pixel apart).

We have filed bug 766937 to add scrollMaxX and scrollMaxY properties on elements, so that you should be able to do "element.scrollMaxX > 0 || element.scrollMaxY > 0" instead. However, it will probably be a while before that makes it out in release builds.
[ Reply to this ]
Posted by avih at 2012-06-22 03:14:03
1. Your sample code doesn't produce a scrollable div (even if I add more content) - Tested on Firefox 13.01 and on nightly.

2. Your patch seems to return the behavior of Gecko pre v1.7 - many years ago (see code snippest at the bottom), So it might be worth checking why was it changed for 1.7 to make sure current change doesn't introduce a regression (I don't know the exact commit/bug which changed it).

3. It seems (assuming the code below is correct) that the body element was an exception on pre 1.7 Gecko. How about it now?

4. So, what is the FULLY working method to detect a scrollable node, without exceptions?

That's SmoothWheel's current detection of a scrollable node ("1.6+" at the comment means 1.7 or later):

realHeight = currNode.clientHeight;

//avih: modified hack for FB 0.8+ MOZ 1.6+ etc from AIO v0.96
if (!sw_geckoPre_1_7 || currNode == bodyEl)
realHeight += sw_aioGetStyle(currNode, "border-top-width") + sw_aioGetStyle(currNode, "border-bottom-width");

if (currNode.scrollHeight>realHeight && //standard test
realHeight*1.2>currNode.offsetHeight && (realHeight+40)>currNode.offsetHeight)//avih: ugly hack sanity
return currNode;
[ Reply to this ]
Posted by stak at 2012-06-22 08:51:28
1. Correct, it's not meant to. It has a display: table set on it, which makes the overflow property essentially moot. However, that is the scenario (a non-overflowable element) for which my patch changes behaviour.

2. Thanks for pointing that out, I'll try to find that out.

3. The body element is not an exception with new code. That is, if in my example code you replace "div" with "body" the scrollWidth/scrollHeight still change from 110 to 100 with the patch.

4. Here's a fully working method without exceptions:

currNode.scrollTop = 1;
var scrollable = (currNode.scrollTop == 1);
currNode.scrollTop = 0;
return scrollable;


Of course, this has the side-effect that the element (if scrollable) will be scrolled by a pixel and then scrolled back. But it will always return the right answer, no exceptions, and is probably 100% cross-browser compatible to boot (although I haven't tested it).

The code we are currently using in the Fennec front-end to do this check is at https://hg.mozilla.org/mozilla-central/file/aaa6906f8e92/mobile/android/chrome/content/browser.js#l3261 - and that's not perfect either, as it doesn't take into account rounding for subpixel layouts.
[ Reply to this ]

[ Add a new comment ]

 
 
(c) Kartikaya Gupta, 2004-2024. User comments owned by their respective posters. All rights reserved.
You are accessing this website via IPv4. Consider upgrading to IPv6!