Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.0.3, 2.1.4, 2.2.1
-
Component/s: Comments
-
Testing Instructions:
-
Affected Branches:MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
-
Fixed Branches:MOODLE_21_STABLE, MOODLE_22_STABLE
-
Pull from Repository:
-
Pull Master Branch:
MDL-28952-master -
Pull Master Diff URL:
Description
Prerequisite:
- Comments are enabled in moodle on system level.
- There is a course on http://MYMOODLEDOMAIN/course/view.php?id=COURSEID with comments block added
- Some comments have been added in the comments block
In the comments block, there are links to each commenting user's profile of this type:
http://MYMOODLEDOMAIN/user/view.php?id=USERID&course=COURSEID
But: "&" should be "&".
Now, if a user clicks this link, the course parameter isn't recognized and the user is forwarded to
http://MYMOODLEDOMAIN/user/profile.php?id=USERID
which he may or may not be able to view.
Replacing in /comment/lib.php, line 598
$c->profileurl = $url->out();
with
$c->profileurl = $url->out(false);
solves the problem, the user is shown view.php as desired, but I don't know which side-effects this has.
Gliffy Diagrams
Issue Links
Activity
I just verified that the problem is still present in 2.2.x
I would be grateful if the small patch could be included into moodle core
$url->out should really be used only when we have to print the output
up for review
Thanks
Looks good Ankit.
+1 for integration.
Thanks Rosie for the review.
Sending for integration
Thanks
Hi guys,
There is a bit of an issue here as you are changing the result that is returned from the get_comments method (comments API).
Normally we would only allow such a change in master as it backporting it to stable may cause peoples own code to break.
If there's a good reason to backport then it can be considered. Either way I think the original solution Alexander suggested may be the safer approach for STABLE branches.
(For instance Moodle's plugins database uses this API and would be affected)
I've added Dongsheng as a watcher on this issue. Dongsheng would you mind having a look at this and seeing what you think about changing the result of the get_comments API.
Is it a good idea or do you think we should avoid it.
Cheers
Sam
Hello all
I think using `$c->profileurl = $url->out(false);` makes sense. I cannot see problems of fixing & in url, after all it's the expected result, is it?
Hi Dongsheng,
$url->out(false) I think is certainly the fix for the stable branches.
But for the master branch what do you think about changing profileurl from a string, to a moodle_url object?
(sorry I could have been clearer in my earlier question)
Cheers
Sam
Hi Sam
I suggest to use $url->out(false).
The return value from get_comments will be used by json_encode, thus moodle_url::__toString will be called, currently this method is using moodle_url::out(true)
Awesome thanks DS, that clears it up nicely!
All up to you now Ankit.
Thanks guys for the feedback,
Changes made, submitting again for integration.
Thanks
Thanks Ankit, this has been integrated now 
Tested on branches 2.1, 2.2 and master. The link works properly now.
Thanks.
Congratulations are in order, you've made it, or at least your code has!
It's now part of Moodle and both the git and cvs repositories have been updated.
This issue is being marked as fixed and closed.
Thank you.
I have a patch at https://github.com/dbezborodovrp/moodle/commit/e6b79031314e6da13cc645fd5ac08bc64f7e184f , but am not quite sure how to attach patch to this ticket in JIRA.
Sorry, wrong ticket... see MDL-44634
Thanks for reporting this. I was able to replicate the problem.
It looks like this ampersand is being escaped unnecessarily.
The link is correct when it is loaded by Ajax after the comment is added, but it is incorrect when someone else views it or the page is reloaded.