Wikimedia Bugzilla is closed!

Since the div looks like <div lang="en" dir="ltr" class="mw-content-ltr"> I think the better directionality project is to blame for this. CC Robin and Amir.

From the perspective of JS just about nothing has changed.
bodytext has always been in the middle of the #bodyContent with stuff around it.
The only thing that has changed is bodytext is now wrapped in a div.

The only kind of js this should break is something that tries to grab #bodyContent and iterate over childNodes instead of searching recursively.

The fact that bodytext is now in it's own div would be an enhancement that gives js a narrow chance to actually target the bodytext (with something like '#bodyContent .mw-content-ltr, #modyContent .mw-content-rtl').

The bug seams to be asking for an extra id on this new div. In that case this would be an enhancement.

I don't really know what you mean by "searching recursively". My TabbedLanguages script ([[wikt:User:Yair rand/TabbedLanguages2.js]], which will hopefully be enabled by default at some point on a bunch of Wiktionaries), was broken by this. It searched through the childnodes of #bodyContent/variants and sorted the content into divs based on h2s that were immediate children of bodyContent. The only way to make this work now is to either do a lot of navigating through it to find the mw-content, or use a heavy jQuery search for the class. So, from the perspective of JS everything has moved into a div hidden in the middle of nowhere.

(In reply to comment #3)
> I don't really know what you mean by "searching recursively". My
> TabbedLanguages script ([[wikt:User:Yair rand/TabbedLanguages2.js]], which will
> hopefully be enabled by default at some point on a bunch of Wiktionaries), was
> broken by this. It searched through the childnodes of #bodyContent/variants and
> sorted the content into divs based on h2s that were immediate children of
> bodyContent. The only way to make this work now is to either do a lot of
> navigating through it to find the mw-content, or use a heavy jQuery search for
> the class. So, from the perspective of JS everything has moved into a div
> hidden in the middle of nowhere.

jQuery selectors are faster than you think. Especially with class searching being natively implemented in modern browsers.

I don't even know where to begin on what's wrong with that script.

What was the reason you couldn't use bodyContent.getElementByTagName('h2')?

Hm, I thought jQuery selectors work very slowly on browsers that don't support querySelectorAll...

(Not going to respond to the next part, I don't see what could be wrong with the script.)

That's the way it used to work, looking through h2s, ignoring the h2 occasionally inside toc and preview note, and sorting things like that, but it was significantly slower than pulling everything out at once, temporarily holding the contents in a documentfragment, and then sorting through everything outside the display before dumping everything back in.

(In reply to comment #5)
> Hm, I thought jQuery selectors work very slowly on browsers that don't support
> querySelectorAll...
> 
> (Not going to respond to the next part, I don't see what could be wrong with
> the script.)
My eyes were bleeding from looking at soo much dom. But there's a straight equals on className not taking the potential for another class to be added into account (you're going to have a bug if that's used on a page with only hidden categories). And I wish RegExp's constructor didn't have to be abused that way.
I have problems with some of those == conditional. And rather than .live you should probably be using .delegate and not embedding unescaped user specified contents in your jQuery selector. (fun tip, if that code you used .live on had used .append you would have an XSS vector on your hands)

> That's the way it used to work, looking through h2s, ignoring the h2
> occasionally inside toc and preview note, and sorting things like that, but it
> was significantly slower than pulling everything out at once, temporarily
> holding the contents in a documentfragment, and then sorting through everything
> outside the display before dumping everything back in.

That could be a result of the new code using a document fragment but the old code not, rather than anything to do with other queries being slower. Document fragments are fast because they are isolated from the document and when you put stuff into them the browser gets to omit piles and piles of extra work like firing events because you've removed the nodes from the document while you work on them. jQuery even makes good use of them.

Thanks for the tips. Re: bug from pages with only hidden categories, Wiktionary entries aren't allowed to have only hidden categories. And .delegate runs through .live (although using the additional arguments on .live is an improvement, so I've fixed that). Yes, it's probably faster because it's using a document fragment, and document fragments don't have getElementsByTagName functions. Which goes back to the need for the content holder to be accessible from javascript...

(In reply to comment #7)
> Thanks for the tips. Re: bug from pages with only hidden categories, Wiktionary
> entries aren't allowed to have only hidden categories. And .delegate runs
> through .live (although using the additional arguments on .live is an
> improvement, so I've fixed that).
The problem with .live is it broadly attaches an event to the document root when you only need one local to a specific area. That's a waste of events being fired. Additionally because the selector is the query itself jQuery also goes and queries the whole dom for those nodes and never uses them.
Additional arguments on live? Are you trying to make use of arguments defined as internal-only instead of properly using the .delegate interface provided to you?

> Yes, it's probably faster because it's using
> a document fragment, and document fragments don't have getElementsByTagName
> functions. Which goes back to the need for the content holder to be accessible
> from javascript...
DocumentFragment doesn't have a getElementsByTagName. But it's children do. Insert your stuff into a dummy div instead of directly onto the fragment, and poof, getElementsByTagName.

And if you don't want to do that then use `jQuery(fragment.childNodes).find('h2');`.

(In reply to comment #8)
> Additional arguments on live? Are you trying to make use of arguments defined
> as internal-only instead of properly using the .delegate interface provided to
> you?
Hm, I didn't know that those are internal-only. Fixed.
> DocumentFragment doesn't have a getElementsByTagName. But it's children do.
> Insert your stuff into a dummy div instead of directly onto the fragment, and
> poof, getElementsByTagName.
> 
> And if you don't want to do that then use
> `jQuery(fragment.childNodes).find('h2');`.
I'd have to put checks into it to make sure that the h2 is a direct child of mw-content anyway, it would probably be slower, and of course I'd have to rewrite that section of the script... It would probably be possible to make the script work without an ID available and remove whatever bugs, but leaving the direct parent of the actual content of the page inaccessible from JS is generally a very bad idea, in my opinion. (BTW, mw.util.$content is no longer accurate for most purposes.)

I added the ID mw-content-text in r111647.

The mw-content-text ID appears to only directly surround the content when the page is plainly viewed. In preview mode, #mw-content-text includes the old id-less .mw-content-ltr div, which then includes the actual content, so this isn't really fixed... 

Should this bug be re-opened, or should a separate bug be opened for this?