Bug Tracker

Ticket #9109 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

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:
Blocking: Blocked by:

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

Change History

comment:1 Changed 3 years ago by timmywil

  • Owner set to lmk
  • Priority changed from undecided to low
  • Status changed from new to pending
  • Component changed from unfiled to support

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.

comment:2 Changed 3 years ago by lmk

  • Status changed from pending to new

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).

comment:3 Changed 3 years ago by speednet

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.

comment:4 Changed 3 years ago by rwaldron

  • Owner changed from lmk to jaubourg
  • Priority changed from low to blocker
  • Status changed from new to assigned

comment:5 Changed 3 years ago by Nao (wedge.org)

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.

comment:6 follow-up: ↓ 8 Changed 3 years ago by jaubourg

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 ;))

Last edited 3 years ago by jaubourg (previous) (diff)

comment:7 Changed 3 years ago by jaubourg

  • Milestone changed from 1.next to 1.6.1

comment:8 in reply to: ↑ 6 ; follow-up: ↓ 9 Changed 3 years ago by Nao (wedge.org)

Replying to 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.)

comment:9 in reply to: ↑ 8 Changed 3 years ago by jaubourg

@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.

comment:10 Changed 3 years ago by jaubourg

  • Status changed from assigned to closed
  • Resolution set to fixed

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

comment:11 Changed 3 years ago by jaubourg

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

comment:12 follow-up: ↓ 13 Changed 3 years ago by Nao (wedge.org)

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? ;))

comment:13 in reply to: ↑ 12 Changed 3 years ago by jaubourg

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

*waves hand*

:)

comment:14 Changed 3 years ago by Nao (wedge.org)

I shall move along then! °_°

comment:15 Changed 3 years ago by Nao (wedge.org)

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... :-/

comment:16 Changed 3 years ago by Nao (wedge)

Okay, fixed in #9239, apparently around the same time I posted about it... (And once again, the code is growing and growing. Uh.)

Note: See TracTickets for help on using tickets.