In subs.php obExit, should $_SESSION['old_url'] really be updated after and xml request? Or should strpos($_SERVER['REQUEST_URL'], 'xml') === false be added to the list of checks not to update it?
Is there another variable that says where someone came from to an action/page that doesn't get set to xml requests?
Yep, that should be added indeed.
Also, since there are several places where this old_url is set, wouldn't make more sense to create a function setOldUrl or something similar that does that?
Functions are good. I finally had to do code edits to change the way permissions for posting and editing posts worked. action_post calls a checkPermissions function. Post2 does it all inline. And jsmodify does it all inline but mostly they are redundant as it puts most restrictions on the database jquery to basically enforce the permissions it later checks.
Would have been great if they all called the permissions function in Messages.subs and if that had a hook to override them. But I know this is legacy from SMF and will be cleaned up over time.
If you find those kind of things, feel free to report anything you find (or just fix them if you want to give it a try, in that case feel free to ask anything it may be not clear ;)), it would help as future reference for future refactoring of the code! ;D
Along the same lines, jsmodify calls functions like isAllowedTo() which checks permissions and if they fail redirects to an error page. Of course that error page is a standard html page not any kind of response expected over ajax so instead of the user seeing the error it seems like nothing happens.
Note that's different than allowedTo which returns a bool (or equivalent) to the php.
Yeah, another piece that is not very well structured.
I think isAllowedTo (or better fatal_error) should know "how" to setup the answer (i.e. html, xml, json).
But that's something for 1.1, along with some kind of standardization of the answers I think?
/me feels this should be tracked in the 1200 issue (beta 1.1 goals).
Tried twice to post. Post button not working. Trying quick reply.
Odd. Trying normal post again. If the functions are going to live on then yes put on the list to make them output format aware.
Bump because it has been lost for too long.
On the OP, do we need to add ;api as well .. not sure if we ever call that without xml though
You are probably right, at least for consistency.
And tracked that as well: https://github.com/elkarte/Elkarte/issues/2000
I think I may have added that ;api as part of 1.0.3 O:-)
I can't see it in my repo (script.js line 614).
Not the script but in obexit ... forgot about the script :P
Yes the ;api change is there. It's what triggered me to check if the script had been fixed. (I'd pretty much forgotten the bug as well.)