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.

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