Skip to main content

Bug Tracker

Side navigation

#7864 closed enhancement (patchwelcome)

Opened December 29, 2010 07:35PM UTC

Closed February 06, 2011 03:59PM UTC

Last modified February 07, 2011 09:37PM UTC

Resolve "scrollable" root (body or html/documentElement)

Reported by: musicisair Owned by: musicisair
Priority: undecided Milestone: 1.6
Component: unfiled Version: 1.4.4
Keywords: Cc: dmethvin, danheberden
Blocked by: Blocking:
Description

Several places (6, I think) within jQuery perform a check similar to:


documentElement[scrollTop|scrollLeft] || body[scrollTop|scrollLeft]

Adding a flag in jQuery.support for "documentElementScrolls" (or something like that) would be helpful for developers as well as assist in keeping jQuery DRY.

fuse.js has a check for this located at [src/dom/features.js](https://github.com/jdalton/fusejs/blob/master/src/dom/features.js#L138) on line 138 (well, 156 really) which may be useful.

Attachments (0)
Change History (10)

Changed December 29, 2010 09:23PM UTC by danheberden comment:1

Thanks for providing a bug report :) Could you provide some examples of where this is? I see https://github.com/jquery/jquery/blob/master/src/event.js#L516

and while there are other tests involving scrollTop or similar, they are in a different context

Changed December 29, 2010 09:26PM UTC by danheberden comment:2

owner: → musicisair
status: newpending

Changed December 29, 2010 11:54PM UTC by musicisair comment:3

status: pendingnew

I just searched for "body.scroll" in the jQuery 1.4.4 source code. So, if changes have been made to the 1.4.4 scrollTop/Left parts in the repository my numbers will be off.

Either way, it would be nice to be able to check which element is scrollable with a simple var scrollEl = $.support.documentElementScrolls ? "$("html") : $("body");.

Changed December 30, 2010 03:14PM UTC by danheberden comment:4

musicisair, I wasn't just referring to the line numbers, but rather, the actual content of the source. Are you looking at the latest source code in the repo, or just the 1.4.4 release from the website?

As for adding to support, to me, it seems trivial to add something like that to core that could easily be done outside of the application or as a plugin.

Changed December 30, 2010 03:49PM UTC by dmethvin comment:5

status: newpending

Usually the guiding factor for whether something is added to jQuery.support is whether it is required in the core and provides a DRY improvement or performance boost. We often get requests to add support checks for features like SVG for example, but if we do that we'll be adding load-time overhead to every page for the few who need SVG detection. Core doesn't need SVG detection.

With those guidelines in mind, does this still make sense? Can you provide specific line references to the core code that would be improved?

Changed December 30, 2010 04:31PM UTC by musicisair comment:6

_comment0: I was just looking at the 1.4.4 release. It looks like the http://code.jquery.com/jquery-git.js version is the same. \ \ I'm just assuming the following sections can be DRYed with the $.support check. I haven't done any testing whatsoever. They first check if the documentElement has a scroll position and then check if the document.body has a scroll position. \ \ * Lines 2336-2337 \ * Lines 7883-7884 \ * Lines 7958-7959 \ (http://code.jquery.com/jquery-git.js) \ \ Also, finding the document's main scrolling element is not a trivial task. \ You must: \ * insert a block element to the document with a height greater than the document window (so we can scroll it). \ * cache the initial {{{scrollTop}}} position \ * scroll with {{{++scrollElem.scrollTop}}} where {{{scrollElem}}} is either {{{body}}} or {{{documentElement}}} \ * check if {{{scrollElem.scrollTop}}} value has changed \ * reset the {{{element.scrollTop}}} to the original value \ * then finally remove the block element. \ \ Since jQuery is already performing the 2 most expensive operations (adding and removing a div) for the other support tests it should be relatively easy to add this check. \ \ FYI: the reason we can't just do {{{ $("body,html").animate({"scrollTop",500},1000); }}} is because Opera scrolls BOTH elements (or it used to, its been a while since I've tested it). It is also not all that efficient to try to animate both elements unnecessarily. :-)1293726987959857
status: pendingnew

I was just looking at the 1.4.4 release. It looks like the http://code.jquery.com/jquery-git.js version is the same (in regards to the scroll parts).

I'm just assuming the following sections can be DRYed with the $.support check. I haven't done any testing whatsoever. They first check if the documentElement has a scroll position and then check if the document.body has a scroll position.

  • Lines 2336-2337
  • Lines 7883-7884
  • Lines 7958-7959

(http://code.jquery.com/jquery-git.js)

Also, finding the document's main scrolling element is not a trivial task.

You must:

  • insert a block element to the document with a height greater than the document window (so we can scroll it).
  • cache the initial
    scrollTop
    position
  • scroll with
    ++scrollElem.scrollTop
    where
    scrollElem
    is either
    body
    or
    documentElement
  • check if
    scrollElem.scrollTop
    value has changed
  • reset the
    element.scrollTop
    to the original value
  • then finally remove the block element.

Since jQuery is already performing the 2 most expensive operations (adding and removing a div) for the other support tests it should be relatively easy to add this check.

FYI: the reason we can't just do

 $("body,html").animate({"scrollTop",500},1000); 
is because Opera scrolls BOTH elements (or it used to, its been a while since I've tested it). It is also not all that efficient to try to animate both elements unnecessarily. :-)

Changed January 24, 2011 04:40PM UTC by addyosmani comment:7

cc: → dmethvin, danheberden

I'm with danheberden on this one - although there are occasionally features and enhancements that might be trivial to add to core, we have to weigh up just how widely those changes might be used. I think this is an ideal candidate for a plugin.

dmethvin, what do you think about the line refs provided by musicisair?

Changed February 06, 2011 03:59PM UTC by snover comment:8

_comment0: Nobody seems particularly interested in taking charge on this one so I am going to close it as patchwelcome. \ \ As far as “we can’t animate both because Opera scrolls both”—if this was the case then there would be a lot of broken positional code in jQuery, since, as has been pointed out in this ticket, we only use one or the other, not both.1297008016381540
resolution: → patchwelcome
status: newclosed

Nobody seems particularly interested in taking charge on this one so I am going to close it as patchwelcome.

As far as “we can’t animate both because Opera scrolls both”—if this was the case then there would be a lot of broken positional code in jQuery, since, as has been pointed out in this ticket, we only use one or the other, not both. (I’m not saying it’s not true that this is the case, just that if it is, exclusive checks are insufficient.)

Changed February 07, 2011 09:34PM UTC by musicisair comment:9

More info (http://msdn.microsoft.com/en-us/library/ms534393(v=vs.85).aspx):

With Microsoft Internet Explorer 6 and later, when you use the !DOCTYPE declaration to specify standards-compliant mode, this attribute applies to the HTML element. When standards-compliant mode is not specified, as with earlier versions of Windows Internet Explorer, this attribute applies to the BODY element, not the HTML element.

Changed February 07, 2011 09:37PM UTC by musicisair comment:10

Also (http://www.greywyvern.com/code/opera/bugs/htmlbodyOverflow):

From the CSS2 specifications, section 11.1.1: Visual effects, Overflow and clipping, Overflow: (emphasis added)

UAs must apply the 'overflow' property set on the root element to the viewport. When the root element is an HTML "HTML" element or an XHTML "html" element, and that element has an HTML "BODY" element or an XHTML "body" element as a child, user agents must instead apply the 'overflow' property from the first such child element to the viewport, if the value on the root element is 'visible'. The 'visible' value when used for the viewport must be interpreted as 'auto'. The element from which the value is propagated must have a used value for 'overflow' of 'visible'.

Opera [< 11] treats a root element overflow value of auto the same as visible and allows the value applied to the <body> element (hidden) to apply to the viewport [...]