Moodle
  1. Moodle
  2. MDL-23813

Glossary autolink error message in Quiz

    Details

    • Testing Instructions:
      Hide

      0. You need a course with a glossary, with the glossary filter enabled.

      1. Then you need a page where the glossary filter is hilighting a word, but most of the normal JavaScript (e.g. navigation block) is not loaded. From the description of this bug, it seems that attempting a quiz as a student is a good way to get such a page. (Make sure the 'Show blocks during quiz attempt' setting is turned off in the quiz.)

      2. Make sure the glossary word hilighting actually works.

      Show
      0. You need a course with a glossary, with the glossary filter enabled. 1. Then you need a page where the glossary filter is hilighting a word, but most of the normal JavaScript (e.g. navigation block) is not loaded. From the description of this bug, it seems that attempting a quiz as a student is a good way to get such a page. (Make sure the 'Show blocks during quiz attempt' setting is turned off in the quiz.) 2. Make sure the glossary word hilighting actually works.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      2693

      Description

      Glossary autolink error message in Quiz
      TypeError
      Y.JSON is undefined

        Activity

        Hide
        Frank Ralf added a comment -
        Show
        Frank Ralf added a comment - See also http://moodle.org/mod/forum/discuss.php?d=171859
        Hide
        Joseph Rézeau added a comment -

        BUMP! I reported this bug a year ago and it is still not fixed in Moodle 2.1...
        Now the error message I am getting is slightly different from what I reported then, but the Glossary popup windows still does not show up. See attached screenshot.

        Show
        Joseph Rézeau added a comment - BUMP! I reported this bug a year ago and it is still not fixed in Moodle 2.1... Now the error message I am getting is slightly different from what I reported then, but the Glossary popup windows still does not show up. See attached screenshot.
        Hide
        Beermann Antonio added a comment -

        Hi,
        this happens on our testserver 2.1 when a user is clicking in the link.
        If a admin is clicking on the link, it does work.
        Would be great if this can be fixed.
        Or does anyone know a workaround?

        Show
        Beermann Antonio added a comment - Hi, this happens on our testserver 2.1 when a user is clicking in the link. If a admin is clicking on the link, it does work. Would be great if this can be fixed. Or does anyone know a workaround?
        Hide
        f p added a comment - - edited

        In the Quiz module, the json-parse-min library is included for admins but not for students.

        Show
        f p added a comment - - edited In the Quiz module, the json-parse-min library is included for admins but not for students.
        Hide
        Mart Mangus added a comment -

        In the Quiz module, the json-parse-min library is included for admins but not for students.

        Forcing the loading of json-parse-min library by adding the following HTML code into the theme header

        <script type="text/javascript" src="/lib/yui/3.4.1/build/json-parse/json-parse-min.js"></script>
        

        does not make any difference.

        Are You sure, the problem is about loading json-parse-min library?

        Show
        Mart Mangus added a comment - In the Quiz module, the json-parse-min library is included for admins but not for students. Forcing the loading of json-parse-min library by adding the following HTML code into the theme header <script type= "text/javascript" src= "/lib/yui/3.4.1/build/json-parse/json-parse-min.js" ></script> does not make any difference. Are You sure, the problem is about loading json-parse-min library?
        Hide
        f p added a comment -

        Retried on Moodle 2.2.2+ (Build: 20120315)
        Things are different now : json-parse-min library is included even for student accunts.
        There is no error message anymore, BUT the glossary entry is now shown on a new page instead of an info box : the autolinks act as standard links.

        Show
        f p added a comment - Retried on Moodle 2.2.2+ (Build: 20120315) Things are different now : json-parse-min library is included even for student accunts. There is no error message anymore, BUT the glossary entry is now shown on a new page instead of an info box : the autolinks act as standard links.
        Hide
        Huy Hoang added a comment -

        running 2.2.3+ (build 20120612), auto-linking displays "TypeError" on the pseudo-popup (AJAX response from showentry_ajax.php is OK). tracing in Firebug showed that Y.JSON.parse() is undefined. Problem happens to both admins and students.

        Editing the last line of "/filter/glossary/yui/autolinker/autolinker.js" to add 'json-parse' solved the problem:

        ---------------------
        }, '@VERSION@',

        {requires:['base','node','json-parse','event-delegate','overlay','moodle-enrol-notification']}

        );
        ---------------------

        Show
        Huy Hoang added a comment - running 2.2.3+ (build 20120612), auto-linking displays "TypeError" on the pseudo-popup (AJAX response from showentry_ajax.php is OK). tracing in Firebug showed that Y.JSON.parse() is undefined. Problem happens to both admins and students. Editing the last line of "/filter/glossary/yui/autolinker/autolinker.js" to add 'json-parse' solved the problem: --------------------- }, '@VERSION@', {requires:['base','node','json-parse','event-delegate','overlay','moodle-enrol-notification']} ); ---------------------
        Hide
        Joseph Rézeau added a comment - - edited

        Bug reported almost 2 years ago and still not fixed. Should be fixed before 2.3 release IMO.
        Joseph

        Show
        Joseph Rézeau added a comment - - edited Bug reported almost 2 years ago and still not fixed. Should be fixed before 2.3 release IMO. Joseph
        Hide
        Tim Hunt added a comment -

        Huy Hoang's proposed fix looks right to me, so I made git commits. Now someone needs to test these, and write some testing instructions for this bug, then we can submit it for integration.

        Show
        Tim Hunt added a comment - Huy Hoang's proposed fix looks right to me, so I made git commits. Now someone needs to test these, and write some testing instructions for this bug, then we can submit it for integration.
        Hide
        Andrew Davis added a comment -

        The code change looks simple enough. This just needs some testing instructions and it can go for integration.

        Show
        Andrew Davis added a comment - The code change looks simple enough. This just needs some testing instructions and it can go for integration.
        Hide
        Joseph Rézeau added a comment -

        Tested bug fix on Moodle 2.3beta (Build: 20120618) works fine for me.

        Show
        Joseph Rézeau added a comment - Tested bug fix on Moodle 2.3beta (Build: 20120618) works fine for me.
        Hide
        Huy Hoang added a comment -

        hold the horse!

        sorry for the late notice, but I ran into another related bug. The "Y.io" module is called on line 62 of the same file, but the module is not included.
        --------------
        Y.io(fullurl, cfg);
        --------------

        So, the previous change fixed auto-linking of glossary in other mods (quiz, question, ...), but not for the glossary itself. The change should also include "io":

        --------------
        }, '@VERSION@',

        {requires:['base','node','io','json-parse','event-delegate','overlay','moodle-enrol-notification']}

        );
        --------------

        Show
        Huy Hoang added a comment - hold the horse! sorry for the late notice, but I ran into another related bug. The "Y.io" module is called on line 62 of the same file, but the module is not included. -------------- Y.io(fullurl, cfg); -------------- So, the previous change fixed auto-linking of glossary in other mods (quiz, question, ...), but not for the glossary itself. The change should also include "io": -------------- }, '@VERSION@', {requires:['base','node','io','json-parse','event-delegate','overlay','moodle-enrol-notification']} ); --------------
        Hide
        Tim Hunt added a comment -

        Well detected. I will amend the commits, and then submit for integration.

        I am still hoping someone else will write the testing instructions, but if no one else does, I will.

        Show
        Tim Hunt added a comment - Well detected. I will amend the commits, and then submit for integration. I am still hoping someone else will write the testing instructions, but if no one else does, I will.
        Hide
        Joseph Rézeau added a comment -

        @Huy Hoang, I don't understand the need for this new fix, glossary autolinking within the glossary itself does work.

        Show
        Joseph Rézeau added a comment - @Huy Hoang, I don't understand the need for this new fix, glossary autolinking within the glossary itself does work.
        Hide
        Huy Hoang added a comment - - edited

        @Joseph: I don't know much about the glossary structure so I can't explain it. Sometimes clicking the links takes me to showentry.php (actual new page load), most of the times it would send AJAX request to showentry_ajax.php instead.

        The autolinker.js intercepts the "click" on the links and request showentry_ajax.php (by calling Y.io).

        If the script can fails gracefully, you should still be able to get to showentry.php, and thus it works (in a non-AJAX way). What I experienced is that the script failed on calling Y.io but cannot recover, thus, stopped there with the spinning wheel icon.

        Is it working for you as AJAX without Y.io included?

        Show
        Huy Hoang added a comment - - edited @Joseph: I don't know much about the glossary structure so I can't explain it. Sometimes clicking the links takes me to showentry.php (actual new page load), most of the times it would send AJAX request to showentry_ajax.php instead. The autolinker.js intercepts the "click" on the links and request showentry_ajax.php (by calling Y.io). If the script can fails gracefully, you should still be able to get to showentry.php, and thus it works (in a non-AJAX way). What I experienced is that the script failed on calling Y.io but cannot recover, thus, stopped there with the spinning wheel icon. Is it working for you as AJAX without Y.io included?
        Hide
        Joseph Rézeau added a comment -

        Is it working you for you as AJAX without Y.io included?
        Yes.

        Show
        Joseph Rézeau added a comment - Is it working you for you as AJAX without Y.io included? Yes.
        Hide
        Huy Hoang added a comment -

        my bad!

        I'm using a theme that doesn't show the navigation block in the glossary (we are embedding that Moodle course in an external app). The "io-base" module is included in the Navigation block, and thus it works for you but not for me. I tested again with standard theme and it does work as you confirmed.

        I still think that we should include "io-base" into autolinker.js, so that it doesn't rely on other blocks for including the required module. Beside, it doesn't seem to cause any harm to include "io-base" more than once.

        It's your call, Tim! :-D

        thanks, and sorry for the confusion

        Show
        Huy Hoang added a comment - my bad! I'm using a theme that doesn't show the navigation block in the glossary (we are embedding that Moodle course in an external app). The "io-base" module is included in the Navigation block, and thus it works for you but not for me. I tested again with standard theme and it does work as you confirmed. I still think that we should include "io-base" into autolinker.js, so that it doesn't rely on other blocks for including the required module. Beside, it doesn't seem to cause any harm to include "io-base" more than once. It's your call, Tim! :-D thanks, and sorry for the confusion
        Hide
        Tim Hunt added a comment -

        It is probably the case that every other moodle page includes 'io' for other reasons, so the fact that glossary code does not correctly declare its dependencies never actually generates a user-visible error. However, I think it is worth fixing this.

        Show
        Tim Hunt added a comment - It is probably the case that every other moodle page includes 'io' for other reasons, so the fact that glossary code does not correctly declare its dependencies never actually generates a user-visible error. However, I think it is worth fixing this.
        Hide
        Tim Hunt added a comment -

        Commit amended.

        Show
        Tim Hunt added a comment - Commit amended.
        Hide
        Tim Hunt added a comment -

        Rebased and 2.3 branch added.

        Show
        Tim Hunt added a comment - Rebased and 2.3 branch added.
        Hide
        Susan Montgomery added a comment -

        We are running 2.2.3, and the above fix worked for everything except Quiz. We are getting a popup that says 'type error'

        Show
        Susan Montgomery added a comment - We are running 2.2.3, and the above fix worked for everything except Quiz. We are getting a popup that says 'type error'
        Hide
        Tim Hunt added a comment -

        Oh! Can you give us some more details? Which web browser are you using? How are the glossary and quiz set up?

        Show
        Tim Hunt added a comment - Oh! Can you give us some more details? Which web browser are you using? How are the glossary and quiz set up?
        Hide
        Susan Montgomery added a comment -

        This happens in all browsers (Firefox, IE, Safari) everything is default, no special settings that I am aware of. It's just the multiple choice quiz.

        Show
        Susan Montgomery added a comment - This happens in all browsers (Firefox, IE, Safari) everything is default, no special settings that I am aware of. It's just the multiple choice quiz.
        Hide
        Tim Hunt added a comment -

        OK, thanks for the update. One other thing. Which theme are you using?

        Show
        Tim Hunt added a comment - OK, thanks for the update. One other thing. Which theme are you using?
        Hide
        Susan Montgomery added a comment -

        Standard_white. We did some custom colors and logo, but that was it.
        Here's the site ctyonline.org

        Show
        Susan Montgomery added a comment - Standard_white. We did some custom colors and logo, but that was it. Here's the site ctyonline.org
        Hide
        Tim Hunt added a comment -

        My patch is definitely working for me. Here is what I did:

        0. Check I am using the Standard theme.
        1. Make sure glossary auto-link filter is on.
        2. Create a test course.
        3. Create a Glossary in the course.
        4. Add an entry 'Glossary', 'Well, this here is a glossary entry!'.
        5. Go to the question bank.
        6. Create a new description item with text 'I wonder if this Glossary entry will be auto-linked?'
        7. Preview the description. Check the word 'Glossary' is hilighted, and that clicking it works. It does.

        I was testing in 2.3

        In 2.2.x the the glossary auto-linking JavaScript does not work for me at all

        Anyway, I think the changes I made are clearly an improvement, even if they don't fix everything. I don't really have any more time to work on this.

        Show
        Tim Hunt added a comment - My patch is definitely working for me. Here is what I did: 0. Check I am using the Standard theme. 1. Make sure glossary auto-link filter is on. 2. Create a test course. 3. Create a Glossary in the course. 4. Add an entry 'Glossary', 'Well, this here is a glossary entry!'. 5. Go to the question bank. 6. Create a new description item with text 'I wonder if this Glossary entry will be auto-linked?' 7. Preview the description. Check the word 'Glossary' is hilighted, and that clicking it works. It does. I was testing in 2.3 In 2.2.x the the glossary auto-linking JavaScript does not work for me at all Anyway, I think the changes I made are clearly an improvement, even if they don't fix everything. I don't really have any more time to work on this.
        Hide
        Susan Montgomery added a comment -

        The highlighting is working, and it works perfectly everywhere EXCEPT Students in the Quiz.
        When you click on the highlighted word, you get a pop-up with the error message 'Type Error'
        I have checked all settings, and they are correct. Have applied above io etc fix, to no effect.

        Show
        Susan Montgomery added a comment - The highlighting is working, and it works perfectly everywhere EXCEPT Students in the Quiz. When you click on the highlighted word, you get a pop-up with the error message 'Type Error' I have checked all settings, and they are correct. Have applied above io etc fix, to no effect.
        Hide
        Joseph Rézeau added a comment -

        @Susan,
        I am not seeing this problem. The fix does work for me.
        Did you remember to PURGE ALL YOUR CACHES ?

        Show
        Joseph Rézeau added a comment - @Susan, I am not seeing this problem. The fix does work for me. Did you remember to PURGE ALL YOUR CACHES ?
        Hide
        Jean-Michel Vedrine added a comment - - edited

        Hello Susan,Tim and joseph,
        With standard theme, all caches purged and connected as a student no problem, it works for me too.
        Thanks a lot Tim for working on this.

        Show
        Jean-Michel Vedrine added a comment - - edited Hello Susan,Tim and joseph, With standard theme, all caches purged and connected as a student no problem, it works for me too. Thanks a lot Tim for working on this.
        Hide
        Susan Montgomery added a comment -

        I had purged cache, but I rebooted and looks like changes held this time. Testing in all courses, but looks good! Thanks!

        Show
        Susan Montgomery added a comment - I had purged cache, but I rebooted and looks like changes held this time. Testing in all courses, but looks good! Thanks!
        Hide
        Tim Hunt added a comment -

        Ah! So this change requires in increase in version.php in filter/glossary. I think.

        Show
        Tim Hunt added a comment - Ah! So this change requires in increase in version.php in filter/glossary. I think.
        Hide
        Tim Hunt added a comment -

        Commits amended to increase version.php. Thanks to Petr for explaining this to me.

        Show
        Tim Hunt added a comment - Commits amended to increase version.php. Thanks to Petr for explaining this to me.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks! (21, 22, 23 & master)

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (21, 22, 23 & master)
        Hide
        Frédéric Massart added a comment -

        Good job! Tested on 2.1, 2.2, 2.3 and master! Hurray!

        Show
        Frédéric Massart added a comment - Good job! Tested on 2.1, 2.2, 2.3 and master! Hurray!
        Hide
        Sam Hemelryk added a comment -

        Congratulations your code is upstream - gold star for you!

        This issue + 79 others made it in in time for the minor releases.
        Thank you everyone involved for your exuberant efforts.

        Show
        Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

          People

          • Votes:
            9 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: