jquery#add: seems to get items in collection out of order on larger lists

[getify]

Test case (in chrome, canary on mac): ​http://jsfiddle.net/HHL3m/

Notice that the JS list has "Rhonda" as the first item, but the DOM output (in Chrome) is "Clayton", then "Victor", etc (at least for me). Rhonda shows up somewhere like 10 items down the list. And it doesn't appear to be anything about the alphabetical sorting or anything like that.

Tested on Firefox (aurora, mac) worked fine.

Tested on Saf6 on mac, also broken, but in a different order ("Keely", "Sloane", etc) than Chrome.

NOTE: The third example in the "Examples" section of the add() documentation page suggests that you should be able to add() a new element (not yet in a document) to an existing collection, and expect the collection order to be kept in the sensible appending order (not silently re-ordered under the covers).

I realize there are other ways to accomplish what I'm trying to do here in this simple test, but my more complex code called for this approach more naturally (keeping a simple collection around instead of, say, a dummy container element like a <div> with children() in it).

[Rick Waldron]

Confirmed. Seems to appear in 1.9.x and up

[dmethvin]

Did you try jquery-git? gibson042 put in a fix for this, which is caused by Chrome screwing up its document position operators for disconnected elements.

[Rick Waldron]

@dmethvin, I tested against jsfiddle's 2.0.x and 1.9.x... Just looked at edge and its fixed. ​http://jsfiddle.net/rwaldron/HHL3m/1/

...I couldn't find any other tickets filed.

[gibson042]

Duplicate of #13331.

[Rick Waldron]

Haha, I didn't look on the second page because I thought that was too far back. Go me.

[getify]

Is this supposed to be fixed in 1.9.2-pre (from git)? I just tried that version in my app, and I'm still getting bad ordering behavior in chrome.

In addition, I just tried this ​http://jsfiddle.net/rwaldron/HHL3m/1/ (which says it's using jquery-edge) in Chrome (canary, mac), and it's broken.

[dmethvin]

Honestly I'd prefer to put this into the "unsupported behavior" category.

If you want a jQuery object in that order, push the elements onto an array and $(array).

[getify]

Well, I wish that wasn't the case. But... if you're going to move this into unsupported behavior, please change the docs to remove the example that makes it seem like it is supported behavior. A special note that this is what's happened would be nice, too.

[getify]

Also, I just tried what you suggested, @dmethvin, here: ​http://jsfiddle.net/HHL3m/2/ Unless I misunderstood what you meant, that throws a JS error. :/

[gibson042]

Replying to getify:

Also, I just tried what you suggested, @dmethvin, here: ​http://jsfiddle.net/HHL3m/2/ Unless I misunderstood what you meant, that throws a JS error. :/

Close, but don't wrap the array before sending it to .append: ​http://jsfiddle.net/HHL3m/3/

[getify]

thanks for the clarification.

[gibson042]

Replying to getify:

Is this supposed to be fixed in 1.9.2-pre (from git)? I just tried that version in my app, and I'm still getting bad ordering behavior in chrome.

In addition, I just tried this ​http://jsfiddle.net/rwaldron/HHL3m/1/ (which says it's using jquery-edge) in Chrome (canary, mac), and it's broken.

Oh, brother. I'm betting that mkwst's ​fix has exposed us to some unstable sorting, but just to dot the is and cross the tts, can you report both the WebKit version of your browser and the results from pointing it at ​http://swarm.jquery.org/build/jquery/5e29ff7e59a0c97bae02b7512b40486df0f44022/test/index.html?module=Sizzle?

As for support, I really would like to fix this for good. Something like ​https://github.com/gibson042/sizzle/blob/83284b9729f0bbe5138f831f7150db495f796413/sizzle.js#L621-L625 oughtta do it. Collections containing disconnected elements will incur indexOf calls, basically making the WebKit fix universal.

...And all because we fixed .add for ​mixed connectedness.

[getify]

Running the test suite, as requested, in my Chrome canary mac, 2 tests failed:

​http://gyazo.com/7dfffd67b07e120ec9c6fed7530bfdf6

Running it in Saf6, works fine. Also, running the actual test-case of this bug (​http://jsfiddle.net/rwaldron/HHL3m/1/) with jquery-edge works in Saf6 but not Chrome.

Here's my chrome info:

Google Chrome 27.0.1421.0 (Official Build 184293) canary OS Mac OS X WebKit 537.32 (@143784) JavaScript V8 3.17.2 Flash 11.6.602.171 User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/537.32 (KHTML, like Gecko) Chrome/27.0.1421.0 Safari/537.32

[gibson042]

​r143784, huh? That'd do it. At least our unit test is good.

[timmywil]

Replying to gibson042:

As for support, I really would like to fix this for good. Something like ​https://github.com/gibson042/sizzle/blob/83284b9729f0bbe5138f831f7150db495f796413/sizzle.js#L621-L625 oughtta do it. Collections containing disconnected elements will incur indexOf calls, basically making the WebKit fix universal.

We already landed a version of this. Doesn't seem to have done the job.

[timmywil]

Unless...is the difference that you want to remove the support test and open up the fix to all browsers?

[gibson042]

@timmywil exactly. Note that @getify is using Chrome Canary, which has @mkwst's hot-off-the-presses compareDocumentPosition fix. So we're now back to returning 0 for disconnected nodes, which exposes us to unstable native sort algorithms. Forcing stability with the indexOf checks works, so I'd like to do it every time we see a disconnected node, regardless of user agent.

Any objections?

[gibson042]

For the record, this is the ​unstable sorting I'm talking about: ​http://jsfiddle.net/vU2uB/

[timmywil]

Yea, sounds like we don't have much of a choice. My concern here is performance, but this will likely be shorter.

[gibson042]

WebKit ticket: ​https://bugs.webkit.org/show_bug.cgi?id=110718

Edit: disregard; as @getify points out, this is fundamentally a V8 issue, not WebKit.

[getify]

It'd be nice if they did fix it. But I doubt it's gonna get fixed, if this thread is any indication: ​http://code.google.com/p/v8/issues/detail?id=90

FWIW, I have, temporarily, fixed my own "dammit this is not stable" sorting issues in webkit/chrome by using this JS merge-sort algorithm instead of the built-in sort, so maybe jquery could consider the same (even if just temporarily)?

​http://stackoverflow.com/a/7730507/228852

[dmethvin]

It still frustrates me that people are using .add() and expecting it to work like a push() operator. We work hard to keep the set in document order; if someone needs a specific order for cases like this $(array) or $(selector).append(array) works great.

[gibson042]

That's fair, but this is another bed we made for ourselves... multiple times, in fact. :\

[Richard Gibson]

Fix #13505: Yet another detached add patch

Changeset: 44340c889bbf6d78511f27ebef5786c639010a52

[Richard Gibson]

Fix #13505: Yet another detached add patch

Changeset: 6256c614fd36596debbf52ddcc2c9660705b3d8f

[getify]

@dmethvin--

with all due respect, the docs (as I mentioned above) make it seem like you can add a new element *after* an existing element, in a collection, and have it be added to the DOM in that collection order. Nothing about that example says "be careful, these might not end up in the order you expect, depending on which browser you use."

` <body> <p>Hello</p> <script>$("p").clone().add("<span>Again</span>").appendTo(document.body);</script> </body> `

If it's not intended that we should be able to rely on the order of items in a collection made partially or entirely by add(), then please just consider changing the documentation (removing that example) so we don't keep getting the wrong idea.

For my part, I had no idea there was any back-story (as you clearly share) to add(), and I just showed up and tried to use it (for the first time) based on the docs.

[dmethvin]

Sorry if it came off as whining...I guess it was. The issue is that there are multiple conflicting goals here. Intuitively I suspect everyone would like the disconnected element case to have .add() just tack things on the end. But for similar intuitive reasons most people want connected elements to be in document order.

The problem is that the .add() method has no information about which case to expect (or more pathological cases such as a mix of connected and disconnected elements, even elements from different documents). So doing things "the right way" for every case means ugly workarounds and still may not meet everyone's definition of "right".

If we can declare some cases out of bounds, jQuery can stay faster and smaller. There are still several good ways to accomplish the goal, so we haven't made it any worse for the jQuery user. In fact, I suspect the revised code is faster and easier for most people to follow.

I agree the docs are misleading about several of these issues, we've fallen behind a bit on the docs and now that the release is out I'm planning on getting things in sync. We may yet still support set ordering as described there on all browsers, but I think it's a misuse of .add() and again the revised code is better.

[dmethvin]

Bulk update to milestone 1.10/2.0