Moodle
  1. Moodle
  2. MDL-32106

YUI throwing error while adding image from recent files repository

    Details

    • Testing Instructions:
      Hide

      1. Uncheck repositoryallowexternallinks in settings
      2. Open any TinyMCE texteditor and try to insert an image from Server files, make sure it is inserted

      Tester: please replicate specific bug conditions then verify the issue is fixed.

      Show
      1. Uncheck repositoryallowexternallinks in settings 2. Open any TinyMCE texteditor and try to insert an image from Server files, make sure it is inserted Tester: please replicate specific bug conditions then verify the issue is fixed.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      wip-MDL-32106-master
    • Rank:
      38815

      Description

      Under certain conditions (unable to determine what exactly), YUI will throw an error, as is described in the forums on:
      http://moodle.org/mod/forum/discuss.php?d=183588

      Basically the /repository/filepicker.js causes an error on the following line:

      var linkexternal = Y.one('#linkexternal-'+client_id).get('checked');
      
      This Y.one('#linkexternal-'+client_id) seems to be null in some cases. This causes the system to freeze.
      

      Since then, I've been able to create a hack to work around this problem.
      If I hack it to match the following, it does seem to work:

      ...
                          // when image or media button is clicked
                          if ( this.options.return_types != 1 ) {
      //                        var linkexternal = Y.one('#linkexternal-'+client_id).get('checked');
      //                        if (linkexternal) {
      //                            params['linkexternal'] = 'yes';
      //                        }
      //                    } else {
      //                        // when link button in editor clicked
                              params['linkexternal'] = 'yes';
                          }
      

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for working on this.

        Show
        Michael de Raadt added a comment - Thanks for working on this.
        Hide
        Sam Hemelryk 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
        Sam Hemelryk 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
        Aparup Banerjee added a comment -

        Hi Marina,
        the patch looks good.

        However i couldn't replicate the issue. In fact it works with and without the patch for me.

        We need test instructions that basically mention :

        • Replicate error on current moodle version (or without proposed patch).
        • Now try on integration version (or with patch) and verify the issue is fixed.
        • And 3rd and last, now try the normal (not this use case, or another use case) way still works.
        Show
        Aparup Banerjee added a comment - Hi Marina, the patch looks good. However i couldn't replicate the issue. In fact it works with and without the patch for me. We need test instructions that basically mention : Replicate error on current moodle version (or without proposed patch). Now try on integration version (or with patch) and verify the issue is fixed. And 3rd and last, now try the normal (not this use case, or another use case) way still works.
        Hide
        Marina Glancy added a comment -

        Hi Apu,
        just checked out the current master and tried again. I was able to replicate this - when I'm trying to insert an image from Server files or Recent files nothing happens.
        Please make sure you unset the 'repositoryallowexternallinks' and you try to insert image into the editor.
        I tried it in Firefox and Chromium.

        Show
        Marina Glancy added a comment - Hi Apu, just checked out the current master and tried again. I was able to replicate this - when I'm trying to insert an image from Server files or Recent files nothing happens. Please make sure you unset the 'repositoryallowexternallinks' and you try to insert image into the editor. I tried it in Firefox and Chromium.
        Hide
        Aparup Banerjee added a comment -

        Thanks this has been integrated and is ready for testing.

        Show
        Aparup Banerjee added a comment - Thanks this has been integrated and is ready for testing.
        Hide
        Michael de Raadt added a comment -

        Test result: Test passed and lunch eaten

        Replicated bug before testing.

        Tested in 2.1, 2.2 and master. No errors reported in Firebug.

        Show
        Michael de Raadt added a comment - Test result: Test passed and lunch eaten Replicated bug before testing. Tested in 2.1, 2.2 and master. No errors reported in Firebug.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And this has landed upstream, finally! Yay!

        תודה רבה && شكرا جزيلا



        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - And this has landed upstream, finally! Yay! תודה רבה && شكرا جزيلا Closing, ciao

          People

          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: