? Success

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
25 Oct 2016

Pull Request for Issue # .

Summary of Changes

Remove the enclosure, use the concise jQuery ready function syntax, use javascript strict mode.

Testing Instructions

Go to the administrator (Isis) and open your browser's javascript console. You may see errors when performing any action that involves javascript (like using the menu) or just by loading a page.

This PR only exposes these errors. It does not fix them. The fix is in #12544, do not merge this PR. Merge that one.

Documentation Changes Required

Nope

avatar okonomiyaki3000 okonomiyaki3000 - open - 25 Oct 2016
avatar okonomiyaki3000 okonomiyaki3000 - change - 25 Oct 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Oct 2016
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 25 Oct 2016
Category JavaScript Administration Templates (admin)
avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Oct 2016

See #12544 for the fixes.

avatar okonomiyaki3000 okonomiyaki3000 - change - 21 Nov 2016
The description was changed
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 12 Jan 2017

@okonomiyaki3000 Tested like written in instructions, see with or without Patch Errors or not. So don't know what exact to look for.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 13 Jan 2017

@franz-wohlkoenig It should not be difficult to find errors. Any page of the administrator should at least produce: Uncaught SyntaxError: Octal literals are not allowed in strict mode.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 13 Jan 2017

I have tested this item 🔴 unsuccessfully on 1914619

Tested in J3.7.0-alpha2-nightly on different Isis-Menues, got no Uncaught SyntaxError: Octal literals are not allowed in strict mode. With and without PR got Warnings like getAttributeNode() shouldnt used. Use getAttribute().


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12543.

avatar franz-wohlkoenig franz-wohlkoenig - test_item - 13 Jan 2017 - Tested unsuccessfully
avatar okonomiyaki3000
okonomiyaki3000 - comment - 16 Jan 2017

Excellent. I think 'getAttributeNode()' has nothing to do with this file though. Where does your js console say it appears?

The 'octal literals' bug is actually caused by a line like this:

var cls = this.className.replace(/^.(chzn-color[a-z0-9-_]*)$.*/, '\1');

You will notice that there are no octal literals here. The problem is the replacement string. It should be '$1', not '\1'. This line still seems to be in 3.7 so I'm not sure why your browser is letting it go. May I ask which browsers you've tested this in?

Anyway, keep in mind that this PR is not meant to fix anything and is not expected to be merged. It is here as a reference to expose problems which normally go unnoticed because strict mode is not used. The goal of this patch is to make things break. The fixes are in #12544. That is the PR that should actually be merged.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 16 Jan 2017

@okonomiyaki3000 Used Firefox 50.1.0, thanks for explanation of this PR. Will test #12544

avatar FPerisa
FPerisa - comment - 28 Feb 2018

I have tested this item ✅ successfully on 2392574

Now I'm getting Uncaught SyntaxError: Octal escape sequences are not allowed in strict mode.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12543.

avatar FPerisa FPerisa - test_item - 28 Feb 2018 - Tested successfully
avatar Tchangue
Tchangue - comment - 23 Jul 2018

@okonomiyaki3000 could you please look into: #21233 Thanks

avatar okonomiyaki3000
okonomiyaki3000 - comment - 24 Jul 2018

@Tchangue Yes, this PR has errors. That's actually the only point of it. It is not meant to be merged. It meant to expose the errors by invoking strict mode. The errors are actually fixed in #12544.

avatar okonomiyaki3000 okonomiyaki3000 - change - 24 Jul 2018
The description was changed
avatar okonomiyaki3000 okonomiyaki3000 - edited - 24 Jul 2018
avatar okonomiyaki3000
okonomiyaki3000 - comment - 24 Jul 2018

@FPerisa You got errors. Which means the patch works because it's supposed to expose errors. But I think you're supposed to mark it as failed because we actually do not want errors. Yes, it seems to make no sense. The problem is that the errors are there already, they're just not reported because we're not in strict mode. So in fact, this PR should be Failed and the current Joomla code should also be Failed. #12544 is correct.

avatar kneisel kneisel - test_item - 24 Jul 2018 - Tested successfully
avatar kneisel
kneisel - comment - 24 Jul 2018

I have tested this item ✅ successfully on 2392574

I think this issue can be closed together with #12544 because #12543 was only a patch to allow the test of #12544


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12543.

avatar franz-wohlkoenig franz-wohlkoenig - change - 24 Jul 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Jul 2018

Ready to Commit after two successful tests.

avatar roland-d roland-d - change - 24 Jul 2018
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2018-07-24 08:39:39
Closed_By roland-d
Labels Added: ?
avatar roland-d roland-d - close - 24 Jul 2018
avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Jul 2018

@franz-wohlkoenig Do not commit.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Jul 2018

@kneisel you can't have tested this patch successfully. It intentionally has errors.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 25 Jul 2018

@okonomiyaki3000 you mean set RTC back on Pending?

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 25 Jul 2018

sorry, this is closed.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Jul 2018

This is fine, it was closed without merging. As it should be. We're all fine here now.

Add a Comment

Login with GitHub to post a comment