Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35568

Creating new resources via DND in a hidden section sets visibility incorrectly

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Course
    • Labels:
    • Testing Instructions:
      Hide
      • Drag a new file into a visible section
      • Confirm that the new file is visible
      • Hide the section
      • Confirm that both the new file and the section as a whole appear hidden
      • Drag another new file into the now hidden section
      • Confirm that the new file appears hidden
      • Toggle the section visibility
      • Confirm that both new files and the section as a whole appear visible
      Show
      Drag a new file into a visible section Confirm that the new file is visible Hide the section Confirm that both the new file and the section as a whole appear hidden Drag another new file into the now hidden section Confirm that the new file appears hidden Toggle the section visibility Confirm that both new files and the section as a whole appear visible
    • Workaround:
      Hide

      Unhide the section, toggle the resource and then hide the section again.

      Edit the resource's settings and change the visibility there.

      Show
      Unhide the section, toggle the resource and then hide the section again. Edit the resource's settings and change the visibility there.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-35568_dndupload_hidden

      Description

      If a section is hidden, and you drag/drop a new resource into it, the new resource is created with visible of 1.
      When you unhide the section, the resource visibility goes from 1 (visible) to 0 (hidden).

      Since it's not easy to change the visibility of a resource within a hidden section, this could lead to confusion, chaos and eventually anarchy.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Hi, Andrew.

              Could you please expand on this? I tried creating a resource in a hidden section and its default visibility was hidden.

              Or is this a double-negative thing?: "incorrectly sets visibility incorrectly"

              I did note that when using drag-and-drop, resources are always set to visible. Is that what you are referring to?

              Show
              salvetore Michael de Raadt added a comment - Hi, Andrew. Could you please expand on this? I tried creating a resource in a hidden section and its default visibility was hidden. Or is this a double-negative thing?: "incorrectly sets visibility incorrectly" I did note that when using drag-and-drop, resources are always set to visible. Is that what you are referring to?
              Hide
              dobedobedoh Andrew Nicols added a comment -

              Hi, Michael.

              Sorry about that - being a bit rusty at the moment as I've been really busy.
              I've updated the title and description, and have provided test instructions for when the issue is fixed.

              Show
              dobedobedoh Andrew Nicols added a comment - Hi, Michael. Sorry about that - being a bit rusty at the moment as I've been really busy. I've updated the title and description, and have provided test instructions for when the issue is fixed.
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for the additional information.

              Show
              salvetore Michael de Raadt added a comment - Thanks for the additional information.
              Hide
              rex Rex Lorenzo added a comment -

              Shouldn't this be raised a security issue? With us promoting drag and drop as the best way to upload files, we have run into problems in which a user can drag and drop files into a hidden section via drag and drop, but the files are visible in the resource listing page: /mod/resource/index.php.

              This caused some sensitive content being released.

              Show
              rex Rex Lorenzo added a comment - Shouldn't this be raised a security issue? With us promoting drag and drop as the best way to upload files, we have run into problems in which a user can drag and drop files into a hidden section via drag and drop, but the files are visible in the resource listing page: /mod/resource/index.php. This caused some sensitive content being released.
              Hide
              salvetore Michael de Raadt added a comment -

              Hi, Rex.

              This is a bug in behaviour, but I don't think this is a security issue.

              Thanks for launching the linked issue.

              Show
              salvetore Michael de Raadt added a comment - Hi, Rex. This is a bug in behaviour, but I don't think this is a security issue. Thanks for launching the linked issue.
              Hide
              davosmith Davo Smith added a comment -

              The attached patch checks the section visibility and uses it to set the module visibility (but it always sets the 'visibleold' to '1', so that the resource will unhide when the section is unhidden).

              BTW, whilst working on this, I noticed that creating activities/resources in a hidden section defaults to 'hidden' with 'visibleold' set to hidden as well - there doesn't seem to be a way to create a resource in a hidden section that will unhide when the section is unhidden (other than, with this patch, via drag and drop).

              Show
              davosmith Davo Smith added a comment - The attached patch checks the section visibility and uses it to set the module visibility (but it always sets the 'visibleold' to '1', so that the resource will unhide when the section is unhidden). BTW, whilst working on this, I noticed that creating activities/resources in a hidden section defaults to 'hidden' with 'visibleold' set to hidden as well - there doesn't seem to be a way to create a resource in a hidden section that will unhide when the section is unhidden (other than, with this patch, via drag and drop).
              Hide
              damyon Damyon Wiese added a comment -

              This patch cleanly cherry-picks back to 23 (no drag and drop in 22).

              Show
              damyon Damyon Wiese added a comment - This patch cleanly cherry-picks back to 23 (no drag and drop in 22).
              Hide
              damyon Damyon Wiese added a comment -

              I linked a new issue for the 'visibleold' problem you found Davo (I agree it should work like it does in this patch).

              Show
              damyon Damyon Wiese added a comment - I linked a new issue for the 'visibleold' problem you found Davo (I agree it should work like it does in this patch).
              Hide
              damyon Damyon Wiese added a comment -

              Peer Review Checklist:

              [W] Syntax - Very minor comment issue - comments missing fullstops.
              [-] Output
              [Y] Whitespace
              [-] Language
              [-] Databases
              [Y] Testing
              [-] Security
              [-] Documentation
              [Y] Git - (Cleanly cherry picks back to 23)
              [Y] Sanity check

              Thanks Davo - Sending for integration review.

              Show
              damyon Damyon Wiese added a comment - Peer Review Checklist: [W] Syntax - Very minor comment issue - comments missing fullstops. [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git - (Cleanly cherry picks back to 23) [Y] Sanity check Thanks Davo - Sending for integration review.
              Hide
              damyon Damyon Wiese added a comment -

              Sorry - pushed wrong branch.

              Show
              damyon Damyon Wiese added a comment - Sorry - pushed wrong branch.
              Hide
              damyon Damyon Wiese added a comment -

              Actually - there is a problem with this.

              It only adds the dimmed class to the resource - it does not update the show/hide links etc.

              See toggle_hide_resource_ui in course/yui/toolboxes/toolboxes.js for the full list of what needs to be done (really should call that function).

              Regards, Damyon

              Show
              damyon Damyon Wiese added a comment - Actually - there is a problem with this. It only adds the dimmed class to the resource - it does not update the show/hide links etc. See toggle_hide_resource_ui in course/yui/toolboxes/toolboxes.js for the full list of what needs to be done (really should call that function). Regards, Damyon
              Hide
              davosmith Davo Smith added a comment -

              Damyon - I'm not quite sure what is missing from my code.

              As far as I can tell the steps that are needed are:

              1. Add the 'dimmed' class - done by my javascript
              2. Change the link + alt tag on the show/hide icon - not needed as the the action icons were generated on the server with the correct status
              3. Cope with conditional hiding of icons - not an issue as a newly drag & dropped resource can never have conditional hiding applied (as there has been no opportunity to open up the settings and apply this)
              4. Toggle the availability icon status - again, not an issue as custom availability could not have been configured for a newly drag and dropped icon.

              Please let me know if I've missed some subtlety in the code that I haven't already accounted for.

              Show
              davosmith Davo Smith added a comment - Damyon - I'm not quite sure what is missing from my code. As far as I can tell the steps that are needed are: 1. Add the 'dimmed' class - done by my javascript 2. Change the link + alt tag on the show/hide icon - not needed as the the action icons were generated on the server with the correct status 3. Cope with conditional hiding of icons - not an issue as a newly drag & dropped resource can never have conditional hiding applied (as there has been no opportunity to open up the settings and apply this) 4. Toggle the availability icon status - again, not an issue as custom availability could not have been configured for a newly drag and dropped icon. Please let me know if I've missed some subtlety in the code that I haven't already accounted for.
              Hide
              damyon Damyon Wiese added a comment -

              MDL-37430 includes a change to make toggle_hide_resource_ui easier to call. Suggest we wait for that (or base this on top of that patch).

              Show
              damyon Damyon Wiese added a comment - MDL-37430 includes a change to make toggle_hide_resource_ui easier to call. Suggest we wait for that (or base this on top of that patch).
              Hide
              damyon Damyon Wiese added a comment -

              Sorry Davo - I thought the show/hide icons would need updating but you're right it does work.

              Thanks - sending for integration now.

              Show
              damyon Damyon Wiese added a comment - Sorry Davo - I thought the show/hide icons would need updating but you're right it does work. Thanks - sending for integration now.
              Hide
              stronk7 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
              stronk7 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
              samhemelryk Sam Hemelryk added a comment -

              Thanks guys this has been integrated now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks guys this has been integrated now.
              Hide
              fred Frédéric Massart added a comment -

              Test successful on master. Thanks!

              Show
              fred Frédéric Massart added a comment - Test successful on master. Thanks!
              Hide
              poltawski Dan Poltawski added a comment -

              Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

              Show
              poltawski Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    11/Mar/13