Export entry to main glossary crashes

Reproduced on the 2.2 dev beta I was using for Q&A. I'm not sure that the export part matters; I got the crash right after turning comments on and going back to the secondary glossary. Here's the stack trace:

Stack trace:
line 2842 of /mod/glossary/lib.php: comment_exception thrown
line ? of unknownfile: call to glossary_comment_validate()
line 7814 of /lib/moodlelib.php: call to call_user_func_array()
line 852 of /comment/lib.php: call to plugin_callback()
line 863 of /comment/lib.php: call to comment->validate()
line 463 of /comment/lib.php: call to comment->can_view()
line 1169 of /mod/glossary/lib.php: call to comment->output()
line 1202 of /mod/glossary/lib.php: call to glossary_print_entry_icons()
line 18 of /mod/glossary/formats/dictionary/dictionary_format.php: call to glossary_print_entry_lower_section()
line 926 of /mod/glossary/lib.php: call to glossary_show_entry_dictionary()
line 480 of /mod/glossary/view.php: call to glossary_print_entry()

Okay, so I think I see what's going on. When you export an entry to the main glossary its glossaryid gets updated and the sourceglossaryid field is set so that the entry appears in both places. The comment logic always loads the context for the main id, so that you when you try to load an exported entry in the secondary glossary it will always fail the context check.

I think the solution is to test whether there's a source id and if the calling module matches and to load that glossary instead for the context check if so. This does mean that there are two sets of comments (on both glossaries where the entry appears); I don't know if that's the desired behavior. This patch feels hackish but I'm not sure how to streamline it without local variables.

Patch: https://github.com/mackensen/moodle/compare/master...MDL-29848.

I can confirm that this patch works on current MOODLE_21_STABLE.

Stealing this from Eloy and reviewing my older patch; hopefully no hard feelings .

Thanks Charles, the changes look spot on so I'm putting this up for integration review now.

Cheers
Sam

Some hours ago...

the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Thanks, this fix has been integrated into 21, 22 and master.
All good to test.
ps:I've fixed up a minor whitespace

All good Charles, thanks for the fix

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.