Skip to content

JhtmlBehavior - improve _getJSObject #1434

Closed
wants to merge 4 commits into from

4 participants

@okonomiyaki3000

This function has several flaws which I think I've solved. I think that with a little more work I can eliminate it entirely as json_encode() should be used whenever possible.

okonomiyaki3000 added some commits Aug 6, 2012
@okonomiyaki3000 okonomiyaki3000 Fixed several flaws in JHTMLBehavior::_getJSObject:
1) fullScreen was used whenever !is_null($array['fullScreen']) (in otherwords, even if set to false)
2) if $array contained keys 'fullScreen' and 'size', the result was invalid js
3) JSON keys not quoted (possible unexpected behavior)
2055f2e
@okonomiyaki3000 okonomiyaki3000 Removed trailing whitespace 5e9e8a5
@okonomiyaki3000

I changed my mind about removing it entirely. There are too many cases of things being passed with the \ prefix. That practice would need to be deprecated first. It could, perhaps, be purged from all Joomla code but there's no way to know if 3rd parties are making use of it.

In any case, it does not affect this patch as the \ behavior is still supported here.

@realityking
Joomla! member

Why would we remove the use of \? The whole reason that method exists is to support data types beyond those that are handled by JSON.

@okonomiyaki3000

In my opinion, there are better ways to support types such as functions while letting json_encode take care of whatever it possibly can. But I suppose the \ solution is one that has worked well and maybe shouldn't be changed.

@LouisLandry

Good cleanup. If you can rebase it so that it merges cleanly I'll get it merged in this weekend. Thanks a bunch Elijah.

@okonomiyaki3000

Sorry, I'm on Japan time. It'll have to wait till Monday.

eddieajau and others added some commits Aug 19, 2012
@eddieajau eddieajau Merge pull request #1473 from elinw/patch-10
Fix a grammar mistake in a comment
4999d18
@okonomiyaki3000 okonomiyaki3000 cleanup for merge dcb1f90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.