Moodle
  1. Moodle
  2. MDL-44416

Filepicker pagination in detail (table) view does not display second (or subsequent) pages

    Details

    • Testing Instructions:
      Hide

      Enable the Wikimedia repository if it is not already.
      Navigate to a course in which you have edit capabilities (i.e. editingteacher role or higher privilege)
      Click the 'Turn editing on' button
      In the 0th (summary) section, click the cog/sprocket image link to edit the summary.
      The HTML editor should appear. Locate and click on the 'Insert Moodle media' button in the TinyMCE button pallette.
      Click the 'Find or upload a sound, video or applet...' link
      The 'File picker' dialog will appear. On it, click the middle view mode button (set of three in top right corner) to change mode to 'Display folder with file details'
      Click on the 'Wikimedia' repository button.
      Enter the search term 'appalachian state' and click submit.
      Verify the (highlighted) view mode is set to detail view mode.
      Click the scroll bar to initiate additional page fetches.
      The scroll bar thumb will move up and down several times as additional page fetches are executed.
      There are displayed in the list only the initially fetched 24 entries.

      After the patch is applied, the browser cache is cleared, the server caches are cleared, etc. (so as to insure the new javascript file is referenced by the browser), following the same testing steps should yield a displayed list exceeding the first 24 entries.

      Show
      Enable the Wikimedia repository if it is not already. Navigate to a course in which you have edit capabilities (i.e. editingteacher role or higher privilege) Click the 'Turn editing on' button In the 0th (summary) section, click the cog/sprocket image link to edit the summary. The HTML editor should appear. Locate and click on the 'Insert Moodle media' button in the TinyMCE button pallette. Click the 'Find or upload a sound, video or applet...' link The 'File picker' dialog will appear. On it, click the middle view mode button (set of three in top right corner) to change mode to 'Display folder with file details' Click on the 'Wikimedia' repository button. Enter the search term 'appalachian state' and click submit. Verify the (highlighted) view mode is set to detail view mode. Click the scroll bar to initiate additional page fetches. The scroll bar thumb will move up and down several times as additional page fetches are executed. There are displayed in the list only the initially fetched 24 entries. After the patch is applied, the browser cache is cleared, the server caches are cleared, etc. (so as to insure the new javascript file is referenced by the browser), following the same testing steps should yield a displayed list exceeding the first 24 entries.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull 2.6 Branch:
    • Pull Master Branch:

      Description

      When using the filepicker in detail (table, mode = 3) view, when the second page of data is fetched from a repository (due to scroll), the additional items are not displayed.

      If you then click on one of the other diplay modes, the items are displayed along with the first page items, and when you click to return to detail view mode, all of the items are displayed there as well.

      The problem is that the first page's items are set up correctly in the initialize_table_view() function when passed to the YUI tableview's constructor, but when the second or subsequent page's data items are fetched, they are only added to the cached fileslist var, but never added to the tableview's data property.

      When the view mode is changed, the additional items get picked up because they are then already in the fileslist cache, and so are added to the newly created tableview when its constructor is called.

      Related MDL-26832

      The attached patch, and submitted github diff/branch URLs show my proposed fix. Just need to make a call to tableview.data.add() in the append_files_table() for each of the new items. Rather than duplicate the displayname, isfolder, classname property assignments in both the initialize_table_view() and append_files_table routines, put that one level up so it's done before either of those two routines is called.

      Issue exists in current development branch (26, and 25 also), but these changes are in a section of the file that haven't changed for a while, so the patch will apply cleanly.

      Caveat: Not sure if using .forEach on arrays will be backward compatible enough.. may need to change to use simple index iteration.

        Gliffy Diagrams

          Activity

          Hide
          Jason Fowler added a comment -

          Thanks for reporting and patching this. I will have a look at the patch as soon as time allows. If possible, could you please write some testing instructions to go with it?

          Show
          Jason Fowler added a comment - Thanks for reporting and patching this. I will have a look at the patch as soon as time allows. If possible, could you please write some testing instructions to go with it?
          Hide
          Fred Woolard added a comment -

          I can provide the steps to reproduce the problem, but the reproduction depends on a repository that paginates its results. I came across this issue only because I built a custom repository plugin to fetch external URLs from our campus' streaming media server. Checking the Wikimedia repository plugin, it exhibits the behavior too. I'll update testing instructions.

          Show
          Fred Woolard added a comment - I can provide the steps to reproduce the problem, but the reproduction depends on a repository that paginates its results. I came across this issue only because I built a custom repository plugin to fetch external URLs from our campus' streaming media server. Checking the Wikimedia repository plugin, it exhibits the behavior too. I'll update testing instructions.
          Hide
          Jason Fowler added a comment -

          Thank you Fred, I will look over the patch as soon as I can.

          Show
          Jason Fowler added a comment - Thank you Fred, I will look over the patch as soon as I can.
          Show
          CiBoT added a comment - Results for MDL-44416 Remote repository: https://github.com/appalachianstate/moodle Remote branch MDL-44416 _25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1918 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1918/artifact/work/smurf.html Remote branch MDL-44416 _26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1919 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1919/artifact/work/smurf.html Remote branch MDL-44416 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1920 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1920/artifact/work/smurf.html
          Hide
          Jason Fowler added a comment -

          Hi Fred, I have pulled his patch in to my master branch, and I am not seeing the problem solved.

          Show
          Jason Fowler added a comment - Hi Fred, I have pulled his patch in to my master branch, and I am not seeing the problem solved.
          Hide
          Fred Woolard added a comment -

          Jason, please verify the updated filepicker.js file is being loaded into the browser, i.e. make sure in [Site Admin -> Appearance -> JavaScript & AJAX] the JavaScript caching is off, and to make it simpler I turn off YUI combo loading too. I usually purge all caches [Site Admin -> Development -> Purge all caches] to make sure.

          Next is the browser cache, and of course that depends on which one you're using. But if it's FF, then I'll guess you have FireBug installed, and if so, then it'll be easy to verify by direct inspection that the updated file is being loaded, rather than a cached version.

          Show
          Fred Woolard added a comment - Jason, please verify the updated filepicker.js file is being loaded into the browser, i.e. make sure in [Site Admin -> Appearance -> JavaScript & AJAX] the JavaScript caching is off, and to make it simpler I turn off YUI combo loading too. I usually purge all caches [Site Admin -> Development -> Purge all caches] to make sure. Next is the browser cache, and of course that depends on which one you're using. But if it's FF, then I'll guess you have FireBug installed, and if so, then it'll be easy to verify by direct inspection that the updated file is being loaded, rather than a cached version.
          Hide
          Jason Fowler added a comment -

          Ah, I keep forgetting the MDK toolbar doesn't purge all the caches any more. I had to manually purge the caches. It's working now. Thanks Fred. Submitting for integration.

          Show
          Jason Fowler added a comment - Ah, I keep forgetting the MDK toolbar doesn't purge all the caches any more. I had to manually purge the caches. It's working now. Thanks Fred. Submitting for integration.
          Hide
          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
          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
          CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          Marina Glancy added a comment -

          Thanks Fred, integrated in 2.5, 2.6 and master

          Show
          Marina Glancy added a comment - Thanks Fred, integrated in 2.5, 2.6 and master
          Hide
          Adrian Greeve added a comment -

          Tested on the 2.5, 2.6, and master integration branches.
          Scrolling down in the different views now loads additional images (and displays them).
          Test passed.

          Show
          Adrian Greeve added a comment - Tested on the 2.5, 2.6, and master integration branches. Scrolling down in the different views now loads additional images (and displays them). Test passed.
          Hide
          Marina Glancy added a comment -

          Thanks for your awesome code, it is now part of Moodle. Don't forget to submit another issue next week (and peer review two).

          Show
          Marina Glancy added a comment - Thanks for your awesome code, it is now part of Moodle. Don't forget to submit another issue next week (and peer review two).

            People

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

              Dates

              • Created:
                Updated:
                Resolved: