? Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
20 Oct 2014

Some infos

document.id() The document.id function has a dual purpose: Getting the element by its id, and making an element in Internet Explorer "grab" all the Element methods. It also serves as a shorthand for document.getElementById().
For the full description please read the mootools documentation here: http://mootools.net/docs/core/Element/Element

Now about the part with Internet Explorer:
In IE8 Standards mode, getElementById performs a case-sensitive match on the ID attribute only. In IE7 Standards mode and previous modes, this method performs a case-insensitive match on both the ID and NAME attributes, which might produce unexpected results.
For the reference please visit MSDN: http://msdn.microsoft.com/en-us/library/ie/ms536437(v=vs.85).aspx

Conclusion: Since IE8 all browser have the same result on the method documet.getElementById()

Joomla 3.x supports IE>8

  1. That makes the usage of this legacy mootools dependent code useless
  2. The form validation script is now a jQuery script.

So it doesn’t really makes any sense to have mootools code for submitting a form and jquery code to validate it.
Anyhow this PR will not go to the actual behavior, will just make all the document.id() vanilla (that’s mootools NOT dependent), since there is absolutely NO compatibility issue to do so.

Tests

Because the two methods ARE IDENTICAL it’s more a review for typos, but if the team thinks otherwise then to complete the tests you have to go in every form, button in the back end and check that functionality is still in place.

avatar dgt41 dgt41 - open - 20 Oct 2014
avatar jissues-bot jissues-bot - change - 20 Oct 2014
Labels Added: ?
27ef6de 20 Oct 2014 avatar dgt41 typos
avatar wilsonge
wilsonge - comment - 21 Oct 2014

Can we revert the two changes in FOF please? It's maintained outside of joomla (https://github.com/akeeba/fof) and is designed to have 2.5 and 3.x support

avatar zero-24 zero-24 - change - 21 Oct 2014
Category JavaScript
avatar dgt41
dgt41 - comment - 21 Oct 2014

@wilsonge done!

avatar dgt41
dgt41 - comment - 21 Oct 2014

Another good reference for compatibility is: http://quirksmode.org/dom/core/

screenshot 2014-10-21 17 15 41

avatar wilsonge
wilsonge - comment - 21 Oct 2014

Just in case it's not clear :+1: to the idea!

avatar dgt41
dgt41 - comment - 21 Oct 2014

@wilsonge I thought that this way will be easier to go forward. The old PR was really meshy to test or event realize what was the real purpose. Thanks again for the help with unit testing!

avatar Fedik
Fedik - comment - 21 Oct 2014

:+1:
only with pull #4517 can be conflict, depend what will be merged first :smile:

avatar dgt41
dgt41 - comment - 21 Oct 2014

@Fedik Since your PR is already RTC, I removed the changes from here, so no conflicts anymore ????

9e03973 22 Oct 2014 avatar infograf768 cs
avatar dgt41
dgt41 - comment - 22 Oct 2014

Did I manage to destroy this PR with the rebase?

avatar wilsonge
wilsonge - comment - 22 Oct 2014

Ummm I'm afraid so :(

avatar dgt41 dgt41 - close - 22 Oct 2014
avatar dgt41
dgt41 - comment - 22 Oct 2014

Good think I had stashed the changes :)
Closing this one

avatar dgt41 dgt41 - close - 22 Oct 2014
avatar dgt41 dgt41 - change - 22 Oct 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-10-22 09:57:04
avatar dgt41 dgt41 - head_ref_deleted - 8 Nov 2014

Add a Comment

Login with GitHub to post a comment