DokuWiki

IMPORTANT!

This is the old issue tracking system for DokuWiki. Issues can not be added here anymore. Pleaser refer to https://github.com/splitbrain/dokuwiki/issues for the new system.

IMPORTANT!
Tasklist

FS#2647 - $NS is deleted, but template.php ajax.php mediamanager.php still use it.

Attached to Project: DokuWiki
Opened by furun (furun) - Monday, 22 October 2012, 04:21 GMT
Last edited by Andreas Gohr (andi) - Sunday, 20 October 2013, 19:46 GMT
Task Type Bug Report
Category Backend
Status Unconfirmed
Assigned To No-one
Operating System All
Severity Medium
Priority Normal
Reported Version 2012-10-13 "Adora Belle"
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

not sure about this...

$NS is deleted, but template.php, media.php, ajax.php, mediamanager.php still use it (and maybe plugins). (unused declaration in media.php.)

is this intended?

(some of the functions are defining $NS, but i think not that all functions are using $NS correctly. correct $NS should be renamed to $ns or a alternative global name to avoid conflicts with the deprecated $NS, and all functions should by checked if they should use global $NS. plugin developers should be informed if there plugins are using $NS.)
This task depends upon

Comment by Anika Henke (ach) - Monday, 22 October 2012, 09:27 GMT
Hmm, I was the one who deleted it. Sorry, I didn't check if it was in use, because I assumed whoever deprecated it would have made sure of that. (Note to self: Never assume anything!)

The solution should be to use $NS = getNS($ID) instead.
I wonder why that hasn't caused any problems so far!?
Comment by Anika Henke (ach) - Monday, 22 October 2012, 09:54 GMT
Hmm, the original deprecation commit message might shed a light why this doesn't cause any problems:

"$NS is only used in lib/exe/{ajax,mediamanager}.php when no $ID context
is present. The two functions which use $NS in inc/template.php are only
called through those both endpoints, not through doku.php. In doku.php,
$ID is the only correct value, $NS is not kept synchronous with $ID."
See https://github.com/splitbrain/dokuwiki/commit/bced4159c8c5f1b4da2e98b2242c2ec1c09ec74e
Comment by furun (furun) - Monday, 22 October 2012, 10:49 GMT
this was the reason why i was not sure. but some users have reported problems with the media-manager...
maybe anyway rename the global exchange $NS different, because of exactly this confusion? or maybe even clean op the code more deeper?

dont't know if dokuwiki uses a base of all plugins to check incompatibilities, but i expect that some plugins will report problems, and maybe they search for a long time before they find the empty $NS, expect if they check the deprecated/update advices ;)
(for luxury, a regexp search/lister could find plugins and native codes which have to be updated, and put a short advice how. this update had some more changes, and i see more coming.)

PS:
i checked the code, but i don't know this code very well and don't see the dependencies to maybe outside code, so i leave the patching to more skilled coders.
Comment by Michael Hamann (Michitux) - Saturday, 16 February 2013, 12:56 GMT
This seems to be rather complicated as the global $NS is populated from different input variables so it's not possible to replace them easily. However I have the feeling that $NS is defined before it is used as the media manager code includes lib/exe/mediamanager.php which defines $NS and also the ajax handlers define it before calling the media manager functions. I think the proper fix should be to pass $NS as parameter where it is still used.
Comment by Andreas Gohr (andi) - Sunday, 17 February 2013, 20:44 GMT
bad coding style but not a real bug. should be fixed for next release, probably by reorganizing the code

Loading...