Skip to main content

Bug Tracker

Side navigation

#9109 closed bug (fixed)

Opened May 05, 2011 12:57AM UTC

Closed May 09, 2011 07:49AM UTC

Last modified March 08, 2012 08:57PM UTC

support.boxModel now false in IE6 even when not in quirks mode

Reported by: lmk Owned by: jaubourg
Priority: blocker Milestone: 1.6.1
Component: support Version: 1.6
Keywords: Cc:
Blocked by: Blocking:
Description

In version 1.5.2, jQuery.support.boxModel returned true in IE6 when document.compatMode = "CSS1Compat". Now returns false as at jQuery 1.6.

This breaks the offset() function, as it doesn't correctly take scrollTop into account. Which in turn breaks .selectable() in UI which is where I started...

Offending line in 1.6 appears to be 1293 where tests div.offsetWidth === 2

Attachments (0)
Change History (16)

Changed May 05, 2011 01:32AM UTC by timmywil comment:1

component: unfiledsupport
owner: → lmk
priority: undecidedlow
status: newpending

Thanks for taking the time to contribute to the jQuery project! Please provide a reduced test case on http://jsfiddle.net that reproduces the issue experienced to help us assess your ticket.

Additionally, test against the jQuery (edge) version to ensure the issue still exists.

Changed May 05, 2011 02:10AM UTC by lmk comment:2

status: pendingnew

Here is Fiddle:

http://jsfiddle.net/EmYKS/3/

I don't have IE6 on internet machine at work so can't test myself.

Run fiddle. Scrunch browser right down until there is a vertical scrollbar on results if necessary. Scroll down to button. Click. Should give different results depending on scroll bar pos (incorrect behaviour).

Changed May 06, 2011 02:20PM UTC by speednet comment:3

I can confirm, this is major issue that affects BOTH IE6 and IE7. Wrecks absolute positioning based on offsets. Please escalate this issue to BLOCKER.

Changed May 07, 2011 08:03PM UTC by rwaldron comment:4

owner: lmkjaubourg
priority: lowblocker
status: newassigned

Changed May 08, 2011 10:32PM UTC by Nao (wedge.org) comment:5

Confirmed here.

I also fixed it on my side. Basically, what happens is that jQ 1.6 creates an invisible body, but IE6 and IE7 don't seem to like that, and will reply 'false' to every single support.* test done on that fake body (including boxModel.)

I'm sorry for not providing a proper patch, I know nothing about github and such. (First post here for me.) But since it's a short fix, I think it's okay to post it here.

Remove this block at line 1272:

	// We use our own, invisible, body
	body = document.createElement( "body" );
	bodyStyle = {
		visibility: "hidden",
		width: 0,
		height: 0,
		border: 0,
		margin: 0,
		// Set background to avoid IE crashes when removing (#9028)
		background: "none"
	};
	for ( i in bodyStyle ) {
		body.style[ i ] = bodyStyle[ i ];
	}

Replace with this: (and fix subsequent indenting appropriately, of course.)

	body = document.getElementsByTagName("body")[0];

	// Frameset documents with no body should not run this code
	if ( body ) {
		div.style.width = div.style.paddingLeft = "1px";

And finally, remove this block at line 1346:

	body.innerHTML = "";
	document.documentElement.removeChild( body );

And replace with:

		body.removeChild( div ).style.display = "none";
	}

Hopefully this will be enough.

Changed May 08, 2011 11:44PM UTC by jaubourg comment:6

_comment0: I find it a bit hard to swallow, given all unit tests passed in testswarm, there must be something else at work here. \ \ @Nao: You cannot just use the body like this without wrapping it all into a document.ready handler. We use a new body element exactly because of this. \ \ I rather suspect that some of the css rules added to the body are somehow confusing IE (as difficult as it can be... ahem). \ \ @lmk: I'm a bit confused by your fiddle. Where do you set document.compatMode? Or is it one of those wonderfully hidden "gems" of IE's configuration?1304900683291177

I find it a bit hard to swallow, given all unit tests passed in testswarm, there must be something else at work here.

@Nao: You cannot just use the body like this without wrapping it all into a document.ready handler. We use a new body element exactly because of this.

I rather suspect that some of the css rules added to the body are somehow confusing IE (as difficult as it can be... ahem).

@lmk: I'm a bit confused by your fiddle. Where do you set document.compatMode? Or is it one of those wonderfully hidden "gems" of IE's configuration?

(EDIT: OK, stupid me checked the MSDN ;))

Changed May 08, 2011 11:44PM UTC by jaubourg comment:7

milestone: 1.next1.6.1

Changed May 09, 2011 06:22AM UTC by Nao (wedge.org) comment:8

Replying to [comment:6 jaubourg]:

@Nao: You cannot just use the body like this without wrapping it all into a document.ready handler. We use a new body element exactly because of this.

I don't know. My jQ is included in my script at the very end of the page, precisely so I don't have to call .ready(). Anyway, I had the bug-- no quirks or anything, I'm just doing a $(element).offset() at some point, and it's returning the wrong coordinates in IETester with an IE6 or IE8 in IE7 compatibility mode, because support.boxModel is false when it should be true. This doesn't happen in jQuery 1.5.2 or earlier. Actually, calling .offset() on the element's ancestry eventually returns negative values. That should be some kind of hint for jQ ;)

I rather suspect that some of the css rules added to the body are somehow confusing IE (as difficult as it can be... ahem).

As I said, it returns false for everything, it's not just the boxModel value. Hopefully you can reproduce the problem and find a workaround. (If you can't reproduce, someone please leave me your e-mail address or something so I can send you my demo link.)

Changed May 09, 2011 07:07AM UTC by jaubourg comment:9

@Nao:

I'm not discussing the validity of the bug report so no need for a hint there, really ;)

I'm a bit puzzled at the "OH GNO JQWERY BROKE MY APP" vibe to the comments, together with a sadly non-generic enough solution. That's really all I was talking about in response to your comment and it doesn't imply any kind of judgement or anything: I too have to make my apps work if I wanna eat something at the end of the month :)

All I'm saying is that we set visibility to hidden on the body, together with zeroed dimensions in support.js (not your code :P). I'm wondering if it doesn't prevent IE from determining proper css values in the support tests that occur within said body element (which happen to be the boxModel-related tests incidently).

You know: crazy uncle IE not getting his dimensions straight despite its wonderful CSS implementation that prevents it from getting dimensions in visibility hidden elements, or some other non-sense (wild guess) ;)

If we cannot make the new body work, it's simple enough to bring back the document ready block from 1.5.x, though I'm hopeful it won't come to that.

I should get some time today to investigate this more closely.

Changed May 09, 2011 07:49AM UTC by jaubourg comment:10

resolution: → fixed
status: assignedclosed

Fixes #9109. When jQuery is loaded in the body, then the fake body element used in support has to be inserted before the document's body for boxModel to be properly detected (got the hint by looking at the code in jQuery mobile). Test page added so that we can keep checking this.

Changeset: efd0fce7a1ae9bc62ef2b1aa51d7adc224da1ec0

Changed May 09, 2011 07:51AM UTC by jaubourg comment:11

@Nao: can you try http://code.jquery.com/jquery-git.js in your application and confirm/infirm this fixes your issue?

Changed May 09, 2011 08:17AM UTC by Nao (wedge.org) comment:12

Yes, I can confirm it's fixed. Thanks!

(And woohoo, the git version adds an extra 1.5KB to the file size in just one week of work... I wonder what happens when people notice that jQuery 3.0 gzipped+minified is over 200KB? ;))

Changed May 09, 2011 09:32AM UTC by jaubourg comment:13

"These are not the kilobytes you're looking for."

*waves hand*

:)

Changed May 09, 2011 10:09AM UTC by Nao (wedge.org) comment:14

I shall move along then! °_°

Changed May 13, 2011 02:07PM UTC by Nao (wedge.org) comment:15

May I reopen this?

I'm afraid this whole block of code is a bit unfinished as it is.

I started deploying v1.6.1 on my project, and then I realized it caused a glitch at the bottom of the screen. See, I have a margin:1ex on the very last div, so there's a visible 1ex line at the very bottom that's basically outside of the body, and into the html element. Let's call it "outer space". Another way to see that outer space would be to have a HTML page that's smaller than your browser's viewport height.

Now, let's set the body's background color to black for instance. With jQuery 1.5.2 loaded, the outer space shows up in ''black'', as expected (browsers use the body background to fill in for non-body areas, or something.)

With jQuery 1.6 and 1.6.1 loaded, the outer space is ''white''. So it creates a very unpleasing white stripe at the bottom -- it could even fill the entire screen if the body is very small.

I reproduced the problem in Opera 11.50 and IE8 (it actually works as expected in IE7). I suspect other browsers would be the problem, but two browsers is enough for me to point at a potential jQuery issue.

Now, I immediately thought of this bug report. And it does seem that it is related. In Opera 11.50, deleting the documentElement.insertBefore() and documentElement.removeChild(body) lines allowed me to set a desired color on the invisible body -- I set it to background: "!#000" and the problem went away. So that would mean visibility:hidden is not enough to hide the extra body, *and* Opera seems not to comply with jQuery's request to delete the body after tests are made.

Deleting the two lines mentioned above will only help with the problem, but not fix anything magically. Obviously it also breaks .offset() as per the original bug.

Is there any reason why this implementation is preferred to jQuery 1.5.2's? It really works flawlessly for me in all browsers I tested... :-/

Changed May 14, 2011 08:07AM UTC by Nao (wedge) comment:16

Okay, fixed in #9239, apparently around the same time I posted about it...

(And once again, the code is growing and growing. Uh.)