Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up the 'core' javascript file #6512

Closed

Conversation

okonomiyaki3000
Copy link
Contributor

This is the follow up to this here: #6509

Define the Joomla 'core' function inside of a strict closure, fix all the ensuing problems.

  • missing semicolons: fixed
  • undeclared variables: fixed
  • nonsense code: sort of fixed

Testing

These functions are used all over the place and, since the file was not in strict mode before, the mistakes in it were not apparent anyway. So the best way to test is to first try #6509 and see where it goes wrong. There is usually a comment above each function to tell where it is used. These are generally reliable.

There are certainly several functions which are no longer used at all and some which are specifically deprecated. There is some chance that a 3rd party is using them but there should be a plan for removing all the junk.

@okonomiyaki3000 okonomiyaki3000 changed the title Fix up the 'core' javscript file Fix up the 'core' javascript file Mar 20, 2015
@Fedik
Copy link
Member

Fedik commented Jun 14, 2015

@okonomiyaki3000 please check the core.js, there was some changes to Joomla.submitform, and maybe some other ... that cause the merge conflict

@okonomiyaki3000
Copy link
Contributor Author

Uh. Great. These errors have nothing to do with any of these changes. Thanks, Travis.

@okonomiyaki3000
Copy link
Contributor Author

It seems that FOFPlatform (whatever it is) does not implement the method logAddLogger which it is supposed to do. So this causes Travis to fail. Please note that FOFPlatform has nothing to do with the core javascript file which is the only thing modified by this PR.

@Fedik
Copy link
Member

Fedik commented Jun 16, 2015

test works good for me

@Bakual
Copy link
Contributor

Bakual commented Jun 16, 2015

These errors have nothing to do with any of these changes. Thanks, Travis.

Travis recently sometimes stalls because it takes to long to fetch the dependencies with composer.
I've restarted the failing job, hopefully it will pass now.

@anibalsanchez
Copy link
Contributor

#TEST OK

Nice to have a cleaner Javascript implementation.


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

@zero-24
Copy link
Contributor

zero-24 commented Jun 21, 2015

RTC Thanks 😄


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

@zero-24 zero-24 added the RTC This Pull Request is Ready To Commit label Jun 21, 2015
@zero-24 zero-24 modified the milestone: Joomla! 3.4.4 Jul 2, 2015
@Kubik-Rubik
Copy link
Member

Thank you @okonomiyaki3000. I've fixed the conflicts and merged the PR.

@javigomez
Copy link
Contributor

@Kubik-Rubik I think this merge cause this issue: http://issues.joomla.org/tracker/joomla-cms/7388

@Kubik-Rubik
Copy link
Member

@javigomez Thank you, we have to check it.

@okonomiyaki3000 @Fedik @anibalsanchez Could you also take a look?

@Fedik
Copy link
Member

Fedik commented Jul 9, 2015

yes, need some time, and I will make a pull ... if no one will make it faster 😄

@Fedik
Copy link
Member

Fedik commented Jul 9, 2015

@Kubik-Rubik there it is #7391

@Kubik-Rubik
Copy link
Member

@Fedik Wow, that was fast. Thank you, I will check it soon. @javigomez Could you please test this PR #7391 by Fedik?

@okonomiyaki3000
Copy link
Contributor Author

@Fedik @Kubik-Rubik Thanks!

@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015
@okonomiyaki3000 okonomiyaki3000 deleted the fix-core-problems branch January 25, 2016 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants