Moodle
  1. Moodle
  2. MDL-33444

New filepicker ignores labels from repo plugin

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.2
    • Component/s: Filepicker, Repositories
    • Labels:
      None
    • Testing Instructions:
      Hide

      Testing requires a little hacking because this is an interface for 3rd party repositories:

      1. Open filepicker from any filemanager or textarea-insert-image
      2. Select Upload, make sure the label for a file says "Attachment"
      3. Edit file repository/upload/lib.php and change the label in function get_listing()
      4. Repeat steps 1-2 (refresh page) and make sure the label is changed

      Show
      Testing requires a little hacking because this is an interface for 3rd party repositories: 1. Open filepicker from any filemanager or textarea-insert-image 2. Select Upload, make sure the label for a file says "Attachment" 3. Edit file repository/upload/lib.php and change the label in function get_listing() 4. Repeat steps 1-2 (refresh page) and make sure the label is changed
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-33444-master
    • Rank:
      41334

      Description

      The new filepicker in 2.3 uses templates for the forms, but no longer respects the label passed through from the repo plugin - for example:
      /repository/upload/lib.php's get_listing method attempts to set the label as follows:

      $ret['upload'] = array('label'=>get_string('attachment', 'repository'), 'id'=>'repo-form');

      This is ignored by the create_upload_form function in /repository/filepicker.js, which just leaves the label as it is in the template.

      The reason I've noticed this is that my audio recording repository (https://github.com/MaxThrax/moodle-repository_recordaudio) no longer works on 2.3 - because it uses the above label to inject its HTML. If there's proper support for doing this (ideally just adding HTML to the upload form, since I still want to use the standard upload mechanism as I do in the current version of my plugin), please point me in the right direction to do that - I can't see any documentation, and after a brief look at the code, I can't work out how I ought to do it either. I'd love to not need to inject HTML into a label, so please do tell me if there's a better way to do it! (and if there's a way to do it "properly" on Moodle 2.0-2.2 as well, please enlighten me!)

        Issue Links

          Activity

          Hide
          Marina Glancy added a comment -

          MD says: in Moodle 2.3 there is a new functionality where repository can take control over the whole right panel. See Equella plugin for example.

          Show
          Marina Glancy added a comment - MD says: in Moodle 2.3 there is a new functionality where repository can take control over the whole right panel. See Equella plugin for example.
          Hide
          Paul Nicholls added a comment -

          Hi Marina/Martin, where can I find the Equella repository plugin? It doesn't seem to be part of my up-to-date working copy of the master branch, and I don't see it in the plugin repository.
          Cheers,
          Paul

          Show
          Paul Nicholls added a comment - Hi Marina/Martin, where can I find the Equella repository plugin? It doesn't seem to be part of my up-to-date working copy of the master branch, and I don't see it in the plugin repository. Cheers, Paul
          Hide
          Paul Nicholls added a comment -

          Hi Marina (and Martin if you're watching),
          I've just posted a new enhancement request (MDL-33640) to add the ability to make use of custom filepicker templates (particularly in create_upload_form) - as I'm currently using a somewhat dirty workaround to do so (as per that ticket, I didn't want to use the Equella repository's approach of the iframe-like text/html object for reasons outlined in that ticket). I think it'd make it much more flexible with reasonably minimal work (I'd have a crack at implementing it myself if I had more than 2 hours of work time left before I leave the country for the rest of the month!), and would take the 2.3 filepicker changes a small step closer to perfection - the easier it is to create repository plugins that do neat stuff, the more of them we're likely to see...

          -Paul

          Show
          Paul Nicholls added a comment - Hi Marina (and Martin if you're watching), I've just posted a new enhancement request ( MDL-33640 ) to add the ability to make use of custom filepicker templates (particularly in create_upload_form) - as I'm currently using a somewhat dirty workaround to do so (as per that ticket, I didn't want to use the Equella repository's approach of the iframe-like text/html object for reasons outlined in that ticket). I think it'd make it much more flexible with reasonably minimal work (I'd have a crack at implementing it myself if I had more than 2 hours of work time left before I leave the country for the rest of the month!), and would take the 2.3 filepicker changes a small step closer to perfection - the easier it is to create repository plugins that do neat stuff, the more of them we're likely to see... -Paul
          Hide
          Ankit Gupta added a comment - - edited

          Hi,

          I too was working on a similar plugin (A/V Recorder) as Paul (as a part of my GSoC project). I had added the following patch : http://tracker.moodle.org/secure/attachment/28421/add_attachement_label.txt which effectively did this one line using YUI to add the support for the missing attachment label (it's a dirty workaround but not a real solution for reasons aforementioned by Paul).

          this.fpnode.one('.mdl-right').one('*').setContent(data.upload.label);

          As an alternative I had tried other options :
          Using print_login to display the recorders which was clean and set the $action parameter to 'upload' which called the $repo->upload() and uploaded the file to the drafts alright. (https://github.com/ankitdbst/moodle-repository_mediacapture/blob/68d123b68fc7c93f4bbe29cc2682630201149f27/lib.php#L172). Unfortunately I faced some other problems that couldn't be resolved then.

          I second the reasons put forth by Paul for a somewhat simpler solution to the problem which enables adding custom filepicker templates. Giving equella a try might be an option until this happens.

          Show
          Ankit Gupta added a comment - - edited Hi, I too was working on a similar plugin (A/V Recorder) as Paul (as a part of my GSoC project). I had added the following patch : http://tracker.moodle.org/secure/attachment/28421/add_attachement_label.txt which effectively did this one line using YUI to add the support for the missing attachment label (it's a dirty workaround but not a real solution for reasons aforementioned by Paul). this.fpnode.one('.mdl-right').one('*').setContent(data.upload.label); As an alternative I had tried other options : Using print_login to display the recorders which was clean and set the $action parameter to 'upload' which called the $repo->upload() and uploaded the file to the drafts alright. ( https://github.com/ankitdbst/moodle-repository_mediacapture/blob/68d123b68fc7c93f4bbe29cc2682630201149f27/lib.php#L172 ). Unfortunately I faced some other problems that couldn't be resolved then. I second the reasons put forth by Paul for a somewhat simpler solution to the problem which enables adding custom filepicker templates. Giving equella a try might be an option until this happens.
          Hide
          Paul Nicholls added a comment -

          Hi Marina,
          It's nice that you're fixing this regression, but what'd be even better is dealing with the linked issue (MDL-33640) - that way, new plugins (for new versions of Moodle) could be written much more neatly, without needing to use the label as a code injection vector (or to reimplement the standard upload mechanism in order to work using the iframe-like approach used by the new Equella plugin). I'd love to see that dealt with ASAP, so that we can hopefully get quick saturation - the longer we wait, the more people will upgrade to a 2.3 build that doesn't have templates usable by 3rd-party plugins.

          Cheers,
          Paul

          Show
          Paul Nicholls added a comment - Hi Marina, It's nice that you're fixing this regression, but what'd be even better is dealing with the linked issue ( MDL-33640 ) - that way, new plugins (for new versions of Moodle) could be written much more neatly, without needing to use the label as a code injection vector (or to reimplement the standard upload mechanism in order to work using the iframe-like approach used by the new Equella plugin). I'd love to see that dealt with ASAP, so that we can hopefully get quick saturation - the longer we wait, the more people will upgrade to a 2.3 build that doesn't have templates usable by 3rd-party plugins. Cheers, Paul
          Hide
          Ankit Gupta added a comment - - edited

          Hey Paul,

          The current filepicker API has all that we need to make a repository plugin with full capabilites. I have rewritten the plugin (with help from my mentor Rajesh Taneja). It doesn't require any tweaks and workarounds. It's an equella style plugin.

          https://github.com/ankitdbst/moodle-repository_mediacapture

          Also I have used your flash recorder (for audio using flash). So you can take a look. Thanks a lot.

          Show
          Ankit Gupta added a comment - - edited Hey Paul, The current filepicker API has all that we need to make a repository plugin with full capabilites. I have rewritten the plugin (with help from my mentor Rajesh Taneja). It doesn't require any tweaks and workarounds. It's an equella style plugin. https://github.com/ankitdbst/moodle-repository_mediacapture Also I have used your flash recorder (for audio using flash). So you can take a look. Thanks a lot.
          Hide
          Sam Hemelryk added a comment -

          Thanks Marina, this has been integrated now as well.

          Show
          Sam Hemelryk added a comment - Thanks Marina, this has been integrated now as well.
          Hide
          Rossiani Wijaya added a comment -

          This looks good.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This looks good. Test passed.
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          You've made it into the weekly release!

          Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
          http://www.youtube.com/watch?v=_QhpHUmVCmY

          Show
          Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: