This file can be improved in more ways than this. For example, it only turns radio buttons into "button groups" on page load. This means you can't use button groups in repeatable subforms. It always relies on the for/id attributes for relationships between labels and their controls. This means that nesting a control within a label is not sufficient even though it is correct HTML. I plan to fix these problems but not until this file uses strict javascript.

@okonomiyaki3000 is #12543 still valid? I mean both PRs are touching the same file

@dgt41 no. It never was. It exists only to expose the problems. Because javascript will not give any errors unless it's in strict mode, the fixes in this PR fix things that are not apparent except in #12543

This file can be improved in more ways than this.

I can send you the vanilla version for btn-groups id you want, although is just a port

@dgt41 I might want to look at that later. My point was that I'd like to get this PR merged as-is and then work on improving the file in other ways.

I have tested this item ? unsuccessfully on 50717d0

Test on Google Chrome 60.0.3112.101:
Without patch:
No JavaScript error as described here #12543.

With patch:
JavaScript Error while hovering over any MenuItems (admin/isis).
Uncaught ReferenceError: offsetLeft is not defined
at HTMLAnchorElement. (template.js?2fee7f9e62a40e45452f1fad975323bf:193)
at HTMLAnchorElement.dispatch (jquery.min.js?2fee7f9e62a40e45452f1fad975323bf:3)
at HTMLAnchorElement.r.handle (jquery.min.js?2fee7f9e62a40e45452f1fad975323bf:3)

Test on Safari 10.1.2:
Without patch:
No JavaScript error as described here #12543.

With patch:
No JavaScript error as described here #12543.

@icampus

I have tested this item ? unsuccessfully on 50717d0

Operating System

• Joomla! 3.8.0-beta3-dev
• PHP 5.6.2
• MySQLi 5.5.38
• Apache/2.2.29 (Unix)
• Google Chrome 60.0.3112.101
• MacOS 10.12.4

Result similiar to previous test.

Test before patch

• No errors on javascript actions
• Error on load
  Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document. at https://cdncache-a.akamaihd.net/store/:1:2593

Test after patch

• Error on load
  Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document. at https://cdncache-a.akamaihd.net/store/:1:2593
• Error on opening submenus
  template.js?84614c47e342ac58508eacfcd3d2d1ec:193 Uncaught ReferenceError: offsetLeft is not defined at HTMLAnchorElement.<anonymous> (template.js?84614c47e342ac58508eacfcd3d2d1ec:193) at HTMLAnchorElement.dispatch (jquery.min.js?84614c47e342ac58508eacfcd3d2d1ec:3) at HTMLAnchorElement.r.handle (jquery.min.js?84614c47e342ac58508eacfcd3d2d1ec:3)

Same applies to Firefox 55.0.2 on MacOS.

Tested @icampus

@DrDreave Thanks, it looks like more errors had been added while this has been pending. All the more reason we need "use strict".

@okonomiyaki3000 will you be updating this PR then or shall I close it?

Let me have a look. Maybe it can be updated.

I believe there is still some room for improvement here but now it runs in strict mode at least.

I have tested this item ? unsuccessfully on fdd2ac3

In Global Configurations -> SEO Settings:
After the patch Yes is red and No is green. It should be the opposite.

But at least there are no errors in console!

@FPerisa Good catch. Should be OK now.

Rebased with latest staging.

I noticed a block of code that shouldn't have been there. It was doing the same thing as the jQuery 'each' just before it but not taking 'reversed' into account. So it's out. Good riddance.

I have tested this item ✅ successfully on f47ad5d

No errors and expected backend behaviour.

I have tested this item ✅ successfully on f47ad5d

Operating System
Joomla! 3.8.11-dev
Google Chrome 67.0.3396.99
MacOS 10.13.6

Test before patch
No errors on javascript actions

Test after patch
No errors on javascript actions

... so: seems that patch does not harm system

Ready to Commit after two successful tests.

If you cannot replicate the issue that a pr is supposed to be fixing then you can not say that the pr fixes anything

You're right, I could not reproduce the error - I did not find any error before patching (Admin worked "normally" - no javascript error).
There was no error after having installed the patch (Admin worked "normally" - no javascript error).

... so the issue can be closed without patching?
(The problem has been solved "somewhere" in the meantime)

@kneisel The reason you don't see any errors in the existing code is that it is not running in strict mode. You should first test #12543 which is basically the existing code in strict mode. You will see errors with that patch. Then try this patch which is the fix for those errors.

mmmh - I'm a little bit confused. I applied patch #12544 but did not get any javascript error (Chrome 67.0.3396.99)

I think, you mean to apply patch #12543 ?
I did apply patch #12543 and got the error described in #12543 (Uncaught SyntaxError: Octal escape sequences are not allowed in strict mode.)
Than I ADDITIONALY applied patch #12544 and the error disappeared
BUT: patch #12543 differs not only in 'use strict' (administrator/templates/isis/js/template.js)
AND: The actual code base (administrator/templates/isis/js/template.js) has already strict-mode switched on ... without any error.

SO: There is no need to patch it (?)

... sorry - forgot to remove patch ...
will continue testing

I have tested this item ✅ successfully on f47ad5d


I managed to reproduce error by adding 'use strict' to administrator/templates/isis/js/template.js (instead of applying #12543, which did the same)
Than I got: "Uncaught SyntaxError: Octal escape sequences are not allowed in strict mode."

After having applied patch #12544 the error disappeared

(Chrome 67.0.3396.99)

I have tested this item successful as well.

added the milestone - @roland-d please try to remember to add that

@brianteeman Thank you for the reminder, I did forget indeed.

@kneisel Yes, you're right. I had used the wrong id before. Corrected it now.

And, yes, you're right that #12543 has a few other changes from the base code beside just strict mode but they are basically nothing. Such at the removal of a completely unnecessary IIFE. But that has nothing to do with any errors, I promise.