Fix #14084: attach the test div to documentElement, not body.

[mgol]

I refactored support tests so that they don't require body presence. The boxSizing and ajax support tests have been removed as all supported browsers have them true.

Why do we have support.ajax in the first place? Is it because it can be turned off in IE? I don't see why do we keep this test but maybe I'm missing something.

I'll push a separate analogous pull request for 1.x-master as simple cherry-pick won't work in this case.

Once these 2 get merged, I'll work on dividing the support module accross modules that actually use particular tests and then the module will disappear. (I still need to figure out what to do with test/unit/support but that's a separate case).

[mgol]

I also dropped support for Safari 5.1 - we won't release 2.1/1.11 before the middle of September when the next Safari is supposed to come out which means we drop support for 5.1 per our policy.

Some workarounds had to be left because of lovely Android 2.3.

[mgol]

Some tests are failing on Android 2.3 but IIRC so does master. I need to check if I didn't introduce additional failures.

[mgol]

Lovely Android 2.3.3 failures on current master:

EDIT: it seems I didn't introduce additional failures. We can tackle these issues later, though that's sth we should discuss on a meeting...

[timmywil]

I'm concerned about removing unit conversion. Is that not necessary for Safari mobile?

[timmywil]

As in iOS 5.1, which is the latest version an iPad 1/iPhone 4 can have I believe.

[timmywil]

I love the idea of using documentElement and we can certainly merge this first, but it would be great if we could also modularize support and split it up so that removing the CSS module also removes it's support properties (for example).

[mgol]

@timmywil That's the plan, I'm on it, I just wanted to merge this first.

[mgol]

IPhone 4 will even have iOS7. :) But I forgot about the first iPad, meh. :( So we do still need all that Safari 5.1 support?

[dmethvin]

My wife still has an iPad 1, so yes. :smiley_cat:

[mgol]

;) OK, I'll revert the revert.

[mgol]

Support for iOS 5.1 restored.

I'll report a similar pr against 1.x-master on Monday/Tuesday.

[markelog]

@mzgol can you please keep commits atomicity? initially, in this pull request you try to do four different things:

1. Remove support for Safari 5.1
2. Remove jQuery.support.ajax property
3. Fix #14084
4. Refactor support tests

They unrelated and should be done in four/three different commits, preferably in four/three different PR's so discussion about them would not cloud each either. Also would be great if you would have tickets for each of those changes, so they would be reflected in new version changelog and history of the commits, so it could be easily debugged, if needed, and pinpointed in the future.

@rwaldron you usually pretty fanatic about that, aren't you?

About the changes:

• We don't automate tests execution in Android/IE11, before adding them in support tests we probably should add them in testSwarm
• I'm second removal jQuery.support.ajax property but would be great to hear @jaubourg opinion about that
• Why do you want to fix #14084 before #10814? You will have problems with documentElement in oldIE, but while doing #10814, you will, subsequently fix #14084. So why struggle? Why do you want refactor support module twice?

I wouldn't want to land this as is. EDITED

[dmethvin]

In this case we're dealing with some very different things so I agree separate pull requests could be helpful. I think it could theoretically be one pull request, but definitely should be separate commits since they are different subjects. That way we can easily find them and unwind them separately if need be.

[mgol]

You're right, I rushed it too much. I'll break the changes to separate commits after the weekend.

@orkel, I don't see how fixing #10814 will fix #14084, the question if we need to wait for document ready stays.

What are the exact problems with documentElement in oldIE? @gibson042 seemed to be on board with the idea of using documentElement instead of body. Also, this is for master, 1.x-master can always be tackled differently.

[timmywil]

I'm not aware of an issue with using documentElement in oldIE. We use it in Sizzle to attach test elements before doc ready.

[gibson042]

Right, I think we're currently only waiting for ready to guarantee a body, and I'm embarrassed that it took so long to remember the documentElement substitution already in use by Sizzle.

[dmethvin]

Don't worry about it @gibson042, the memory is the second thing to go. In any case, thanks for keeping things moving along!

[jaubourg]

Memory? What's that?

[markelog]

@timmywil, @gibson042 no offence, but i would not give Sizzle as an example of how to deal with oldIE – http://swarm.jquery.org/result/719051 (btw if you like, i could help with that), jQuery also have a problems with couple of oldIE stuff, but i yet only have time to fix test failures for master branch.

Except for unstable line-height test, which i want to remove for two weeks now, but cannot do that until have @gibson042 opinion about that, i can't catch you on IRC :-), maybe we could talk about that on the next meeting? Previously, i also suggested that every commit that caused a test failures should be immediately reverted or presented with a fix.

Okay, back to the discussion – quote from http://msdn.microsoft.com/en-us/library/ie/ms535255(v=vs.85).aspx:

When the !DOCTYPE declaration does not specify standards-compliant mode, and with earlier versions of Windows Internet Explorer, the body object represents the entire surface onto which a document's contents can be rendered.

By "earlier versions" they mean IE6-7, which we call "oldIE" (but without IE8). I can't give you an jsfiddle.net example because this site (or others alike) does not work with oldIE. You have to run the following code on your machine:

div = document.createElement( "div" );
div.innerHTML = "test"
div.style.cssText = "width:100px;zoom:1";

document.documentElement.appendChild( div );
alert( div.offsetWidth ); // 0

This will affect our support test for oldIE, like shrinkWrapBlocks, inlineBlockNeedsLayout and reliableHiddenOffsets. I also should note, that appending DOM-nodes on documentElement for HTML document, is not spec-complaint which does not mean developer could not do it, but like with every hack jQuery does in it source, developer have to have a good reason for it, because browsers could interpret that code in "whatever they want" fashion, especially that nobody would want to do such a thing, except for us.

The good reason, that, on my opinion, we don't have, because #14084 could be easily be fixed at least a two different ways, one of which is doing #10814 because it subsequently will fix #14084.

I don't see how fixing #10814 will fix #14084, the question if we need to wait for document ready stays.

@mzgol If you don't see who one could fix another, then you do it wrong and i'm sorry but that's a fact. Quote from #10814:

• It makes page loads faster.
• No doc ready needed
• No invisible body needed
• It is necessary for supporting Closure

I will say it again:

No doc ready needed

In order to do #10814 you have to accomplish this three:

• Remove jQuery.support properties from public domain, but not jQuery.support object itself
• Dispose support module among other modules
• Make it lazy-load

First two things is just a side effect, because even if we don't want to do them, we could not do the third one without the others, in other words, only last one is the one that really matters. If you don't know how to do it or why it should be like that, oh well... i'm sorry, i'm not trying to be disrespectful or anything, but if it's like that, then this ticket should be assigned on somebody else.

[timmywil]

@orkel

http://swarm.jquery.org/result/719051

These are recent failures that only started occurring when AMD landed (and I plan on fixing them soon). Sizzle is very well-tested against oldIE and I don't think these failures have anything to do with using documentElement (or we would have seen failures before).

Edit: Let me defend Sizzle for a minute so the jQuery team can feel a little more pride about Sizzle. I think it is safe to say that Sizzle is the most stable, cross-browser, consistently performant selector engine out there. Ever since the rewrite, it goes above and beyond it's previous versions and other selector engines (believe me I checked) to fix edge case bugs in all browsers, including oldIE. And much of that is thanks to @gibson042 (and thanks go out to lots of people). There's really no reason not to recommend Sizzle as an example for supporting oldIE. As for the current test failures, they will be fixed, but I suspect they will be fixed on jQuery's end, not on Sizzle's. If I'm wrong about, we will fix it so that Sizzle can continue to be greatly relied upon in oldIE.

[timmywil]

@orkel

Previously, i also suggested that every commit that caused a test failures should be immediately reverted or presented with a fix.

With so many changes to the codebase, failures should be expected, but I agree and we will fix all of the tests before any release.

[timmywil]

BTW, I'm also fine with the lazy-load approach for tests requiring a body. I was actually thinking that's what we would do when we modularized support.

[markelog]

@timmywil i didn't say they are failed because of documentElement, but they failed non the less and only in oldIE some of them before AMD was landed, but perhaps i should phrase myself differently, because i was not implying that something is wrong with Sizzle.

With so many changes to the codebase, failures should be expected

Completely agree, but i would go further:

every commit that caused a test failures should be immediately reverted or presented with a fix.

[markelog]

I'm also fine with the lazy-load approach for tests requiring a body

They do require a body, but thet do not require dom ready

[timmywil]

Completely agree, but i would go further:

every commit that caused a test failures should be immediately reverted or presented with a fix.

I see, I missed "immediately". I would agree for smaller patches as having a clean codebase is a good general rule for development. But like I said, with something this large, some failures are expected. It takes extra work to land something as large as AMD and reverting that would be a pain, unnecessary, and even set us back. First of all, we're not going to release anything that's broken. Second, landing it sooner (even with some faults) is better than waiting until it passes perfectly in every browser because future patches can be based on the new structure without one person having to rebase every patch (and possibly lose stuff). I would just ask everybody to be patient as the codebase moves out of flux.

They do require a body, but they do not require dom ready

Yes, I'm the one who wrote that.

[markelog]

I see, I missed "immediately"...

I propose to not diverge this discussion, that's a different topic, i should not mention it in the first place.

Yes, I'm the one who wrote that.

I know. But i was wanted to point out its significance.

[mgol]

@orkel It's possible to use jsFiddle for testing in every browser. You create a test case using a modern browser and then append /show/light/ to the URL to get the test case in an iframe-free version. E.g. your test: http://jsfiddle.net/m_gol/j9asZ/show/light/

This will affect our support test for oldIE, like shrinkWrapBlocks, inlineBlockNeedsLayout and reliableHiddenOffsets.

I'm aware of the fact we need body for at least some tests, my own test workarounding WebKit zoom problems require body presence. We don't necessarily need to do everything in the same way in 1.x and 2.x branches and this pull request concerns only the master branch.

I also should note, that appending DOM-nodes on documentElement for HTML document, is not spec-complaint which does not mean developer could not do it, but like with every hack jQuery does in it source, developer have to have a good reason for it, because browsers could interpret that code in "whatever they want" fashion, especially that nobody would want to do such a thing, except for us.

That's true but current specs also declare lots of methods how invalid document structure should be parsed/rendered/etc. DOM node constraints don't mention that documentElement can't have more children: http://dom.spec.whatwg.org/#concept-node-tree. Well, this link doesn't explain everything but I wasn't able to find a more concrete one now. @gibson042, @dmethvin, @timmywil, any specs related to how browsers are supposed to interpret that? @orkel, are you sure there is no behavior enforced on browsers in such a case?

I will say it again:

No doc ready needed

In order to do #10814 you have to accomplish this three:

• Remove jQuery.support properties from public domain, but not jQuery.support object itself
• Dispose support module among other modules
• Make it lazy-load

First two things is just a side effect, because even if we don't want to do them, we could not do the third one without the others, in other words, only last one is the one that really matters.

Well, there's always a question: should we lazy-execute all support tests or just some of them? In particular, should we lazy-execute tests not requiring a body? I'm fine either way.

If you don't know how to do it or why it should be like that, oh well... i'm sorry, i'm not trying to be disrespectful or anything, but if it's like that, then this ticket should be assigned on somebody else.

That was a bit harsh, but if the team decides I'd better re-assign it to sb else, I won't object, obviously.

[mgol]

It should be ready to merge now. I've reverted all changes not related to the main point of this pull request, i.e. moving tests out of body to documentElement.

I had to hardcode support.boxSizing to true for that to work, but it'll go away soon anyway.

[mgol]

Per our discussion earlier on IRC, I won't create a version of this pull request against the 1.x-master branch due to problems with oldIE in this approach. The related bug will be fixed when I'll implement lazy-loading of support tests, though for master moving tests to documentElement will help even further.

[markelog]

for master moving tests to documentElement will help even further.

If it does, then i'm all for it, but some things should be mentioned though:

• #14084 ticket should remain open until fix for it will be landed in 1.x-master
• We should not release the new version without coherent fix for 1.x-master
• It would be great if you added test for #14084, you can do that, as you know, through testIframeWithCallback helper

[mgol]

some things should be mentioned though:

• #14084 ticket should remain open until fix for it will be landed in 1.x-master

Agreed, though this pr does fix the issue for master so I'd be for leaving the Fix #14084 text in the commit message. This will auto-close the bug but I'll reopen it immediately after merging.

• We should not release the new version without coherent fix for 1.x-master

I didn't have such plans. :) For 1.x-master lazy-executing support tests will be a fix.

• It would be great if you added test for #14084, you can do that, as you know, through testIframeWithCallback helper

Right, forgot about that.

I'll make the fixes as well as re-exposing jQuery.support probably tomorrow, it seems I won't make it today.

[mgol]

I'll handle both of those issues in pull request #1342, it's getting pointless to separate them (though I do keep separate commits). Closing this one.