Uploaded image for project: '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:

      Description

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

        Gliffy Diagrams

          Activity

          Show
          nakohdo Frank Ralf added a comment - See also http://moodle.org/mod/forum/discuss.php?d=171859
          Hide
          rezeau 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
          rezeau 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
          moddlecamper 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
          moddlecamper 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
          fp2011 f p added a comment - - edited

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

          Show
          fp2011 f p added a comment - - edited In the Quiz module, the json-parse-min library is included for admins but not for students.
          Hide
          mangus 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
          mangus 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
          fp2011 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
          fp2011 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
          sillyxone 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
          sillyxone 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
          rezeau 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
          rezeau 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
          timhunt 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
          timhunt 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
          andyjdavis Andrew Davis added a comment -

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

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

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

          Show
          rezeau Joseph Rézeau added a comment - Tested bug fix on Moodle 2.3beta (Build: 20120618) works fine for me.
          Hide
          sillyxone 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
          sillyxone 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
          timhunt 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
          timhunt 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
          rezeau 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
          rezeau 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
          sillyxone 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
          sillyxone 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
          rezeau Joseph Rézeau added a comment -

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

          Show
          rezeau Joseph Rézeau added a comment - Is it working you for you as AJAX without Y.io included? Yes.
          Hide
          sillyxone 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
          sillyxone 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
          timhunt 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
          timhunt 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
          timhunt Tim Hunt added a comment -

          Commit amended.

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

          Rebased and 2.3 branch added.

          Show
          timhunt Tim Hunt added a comment - Rebased and 2.3 branch added.
          Hide
          suobrien 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
          suobrien 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
          timhunt 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
          timhunt 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
          suobrien 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
          suobrien 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
          timhunt Tim Hunt added a comment -

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

          Show
          timhunt Tim Hunt added a comment - OK, thanks for the update. One other thing. Which theme are you using?
          Hide
          suobrien 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
          suobrien 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
          timhunt 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
          timhunt 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
          suobrien 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
          suobrien 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
          rezeau 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
          rezeau 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
          jmvedrine 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
          jmvedrine 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
          suobrien 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
          suobrien 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
          timhunt Tim Hunt added a comment -

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

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

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

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

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

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

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

          Show
          fred Frédéric Massart added a comment - Good job! Tested on 2.1, 2.2, 2.3 and master! Hurray!
          Hide
          samhemelryk 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
          samhemelryk 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:
                Fix Release Date:
                9/Jul/12