Moodle

Glossary Autolink replace javascript in resources

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.8.5, 1.9.1, 2.1, 2.2
  • Fix Version/s: 2.2
  • Component/s: Glossary
  • Labels:
    None
  • Testing Instructions:
    Hide

    Testing:
    1) Make sure you have turned on the 'Glossary auto linking' under Site administration -> Plugins -> Filters -> Manage filters
    2) Now go to any of your testing course to create a glossary activity. Make sure you have set the 'Automatically link glossary entries' to Yes.
    3) Create an entry called 'alert' in this glossary. Make sure you have ticked the box for 'This entry should be automatically linked'.
    4) Next we need to create a page activity which containing the glossary entry 'alert' in the javascript. To add javascript to the page content, we must first disabled the HTML editor as it automatically removes all the javascript tag when you save the page.
    5) Turn off the HTML editor by going to 'edit my profile'. Under 'When editing text', choose 'Use standard web forms' from the dropdown.
    6) Go to the same course which contains the glossary, create a 'Page activity'.
    7) Fill in the Name and Description fields with anything you like. Under the 'Page content box', copy the code below and click 'Save and display'
    <p>Glossary entry alert should not be autolinked in javascript.</p>
    <script language="JavaScript" type="text/javascript">
    alert ("hello world");
    </script>

    Expected result: When you save and view the page, the alert text ought to display.
    Actual result prior to this fix: The alert does not run. If you view the page source, you will find the alert has been turned into glossary link.

    Show
    Testing: 1) Make sure you have turned on the 'Glossary auto linking' under Site administration -> Plugins -> Filters -> Manage filters 2) Now go to any of your testing course to create a glossary activity. Make sure you have set the 'Automatically link glossary entries' to Yes. 3) Create an entry called 'alert' in this glossary. Make sure you have ticked the box for 'This entry should be automatically linked'. 4) Next we need to create a page activity which containing the glossary entry 'alert' in the javascript. To add javascript to the page content, we must first disabled the HTML editor as it automatically removes all the javascript tag when you save the page. 5) Turn off the HTML editor by going to 'edit my profile'. Under 'When editing text', choose 'Use standard web forms' from the dropdown. 6) Go to the same course which contains the glossary, create a 'Page activity'. 7) Fill in the Name and Description fields with anything you like. Under the 'Page content box', copy the code below and click 'Save and display' <p>Glossary entry alert should not be autolinked in javascript.</p> <script language="JavaScript" type="text/javascript"> alert ("hello world"); </script> Expected result: When you save and view the page, the alert text ought to display. Actual result prior to this fix: The alert does not run. If you view the page source, you will find the alert has been turned into glossary link.
  • Affected Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
  • Fixed Branches:
    MOODLE_22_STABLE
  • Pull from Repository:
  • Pull Master Branch:
    MDL-15377-master
  • Pull Master Diff URL:

Description

Administration > Modules > Filters (Manage Filters for 1.9) > Enable glossary Auto-link (Filer uploaded files: all files for 1.9)
In a course create a glossary with the entry name "else". Enter a description (but not "else"). Choose to automatically link this entry.
In the same course create a file resource. Use the index.html file attached to this issue.
Save.
Open the resource, and view source. "else" keyword from javascript (line 167 in the index.html file) has been replaced by the glossary entry.

Attachments

  1. HTML File
    index.html
    24/Jun/08 10:36 AM
    16 kB
    Jerome Mouneyrac
  2. Text File
    mdl-15377-bug-fix.patch.txt
    24/Aug/09 8:38 AM
    1 kB
    Simon Coggins
  3. Text File
    mdl-15377-filter-phrases-fix.patch.txt
    24/Aug/09 8:38 AM
    4 kB
    Simon Coggins

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Petr Škoda (skodak) added a comment -

hmm, we should start using some javascript highlighting - it seems impossible to parse html properly in PHP only

Show
Petr Škoda (skodak) added a comment - hmm, we should start using some javascript highlighting - it seems impossible to parse html properly in PHP only
Hide
Permalink
Simon Coggins added a comment -

This can be fixed by passing extra parameters to filter_phrases(). As well as excluding <script> tags I'd also suggest excluding <textarea>, and <select> tags as you don't really want auto-linking within them either. See patch mdl-15377-bug-fix.patch.txt.

Filter_phrases() is also used by the other auto-linking filters (mod/data/, mod/resource/, mod/wiki, filter/activitynames) as well as by filter/censor. Since they could potentially lead to a similar bug I would suggest moving the fix to inside filter_phrases(). See patch mdl-15377-filter-phrases.patch.txt (to be applied instead of the first patch).

At present filter_phrases() behaves slightly unintuitively when you pass extra tags to ignore (<a> tags are ignored by default but not if you pass extra tags). In the patch I've added a fifth parameter to default to automatic filtering but allow users to override the default filtered tags.

I've also modified the regular expressions to allow for tags with or without attributes.

These changes shouldn't impact on existing usage of filter_phrases() except to add filtering of script, textarea, and select tags by default.

Show
Simon Coggins added a comment - This can be fixed by passing extra parameters to filter_phrases(). As well as excluding <script> tags I'd also suggest excluding <textarea>, and <select> tags as you don't really want auto-linking within them either. See patch mdl-15377-bug-fix.patch.txt. Filter_phrases() is also used by the other auto-linking filters (mod/data/, mod/resource/, mod/wiki, filter/activitynames) as well as by filter/censor. Since they could potentially lead to a similar bug I would suggest moving the fix to inside filter_phrases(). See patch mdl-15377-filter-phrases.patch.txt (to be applied instead of the first patch). At present filter_phrases() behaves slightly unintuitively when you pass extra tags to ignore (<a> tags are ignored by default but not if you pass extra tags). In the patch I've added a fifth parameter to default to automatic filtering but allow users to override the default filtered tags. I've also modified the regular expressions to allow for tags with or without attributes. These changes shouldn't impact on existing usage of filter_phrases() except to add filtering of script, textarea, and select tags by default.
Hide
Permalink
Sam Marshall added a comment -

We came across this problem at the OU. Ray Guo (one of the developers on my team) has applied one of the patches above to current code, corrected whitespace etc. and put it on his github so that it can be submitted using standard procedure. He has also completed testing instructions which I have added to the bug.

Hopefully with this assistance (on top of the person who actually wrote the code, well done!) we can get this into Moodle 2.2.

Note: When reviewing the code I searched; in core, none of the calls to this function actually specify any arguments except the first two. That means that as best I can see, the behaviour change caused by this function is:

  • All places which use this function previously skipped text instead <head>, <nolink>, and <span class="nolink">.
  • They now additionally skip text inside <script>, <textarea>, <select>, and <a> tags (with any or no attributes).

Second note: I am away the following 2 weeks, sorry if this causes delays.

Show
Sam Marshall added a comment - We came across this problem at the OU. Ray Guo (one of the developers on my team) has applied one of the patches above to current code, corrected whitespace etc. and put it on his github so that it can be submitted using standard procedure. He has also completed testing instructions which I have added to the bug. Hopefully with this assistance (on top of the person who actually wrote the code, well done!) we can get this into Moodle 2.2. Note: When reviewing the code I searched; in core, none of the calls to this function actually specify any arguments except the first two. That means that as best I can see, the behaviour change caused by this function is:
  • All places which use this function previously skipped text instead <head>, <nolink>, and <span class="nolink">.
  • They now additionally skip text inside <script>, <textarea>, <select>, and <a> tags (with any or no attributes).
Second note: I am away the following 2 weeks, sorry if this causes delays.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

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

Show
Eloy Lafuente (stronk7) added a comment - 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
Hide
Permalink
Sam Hemelryk added a comment -

Thanks Sam, this has been integrated to master now.

For those testing this has only been integrated to master as it is an improvement.

Cheer
Sam

Show
Sam Hemelryk added a comment - Thanks Sam, this has been integrated to master now. For those testing this has only been integrated to master as it is an improvement. Cheer Sam
Hide
Permalink
Rajesh Taneja added a comment -

Works Great Sam.
Thanks for fixing this issue

Show
Rajesh Taneja added a comment - Works Great Sam. Thanks for fixing this issue
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

YTC !

(aka, yay, thanks and ciao ) Closing.

Show
Eloy Lafuente (stronk7) added a comment - YTC ! (aka, yay, thanks and ciao ) Closing.

Dates

  • Created:
    Updated:
    Resolved:
    Integration date:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.