Moodle
  1. Moodle
  2. MDL-28284

Filepicker module should be included only where it is needed - not on every page

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Not a bug
    • Affects Version/s: 2.1
    • Fix Version/s: None
    • Component/s: Filepicker
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Go to course front page, view page source, make no filepicker js required, and test other places like module front page
      2. go to pages using editor(forum post), filepicker(course restore) or filemanager(forum post attachment), make filepicker still working

      Show
      1. Go to course front page, view page source, make no filepicker js required, and test other places like module front page 2. go to pages using editor(forum post), filepicker(course restore) or filemanager(forum post attachment), make filepicker still working
    • Affected Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
      git@github.com:dongsheng/moodle.git
    • Pull Master Branch:
      s11_MDL-28284_filepicker_module_master
    • Rank:
      17918

      Description

      Filepicker module should be included only where it is needed - not on every page.
      This presently leads to loading and piles of strings, yui3 modules, and yui2 modules for every page.

      Dongsheng is there anything you can tell me about this - as to why it might be required on every page?
      Certainly if it isn't critical this is some low hanging fruit for a performance win.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi DS , do you mind having a quick look at this issue in case you know something about it.

          Show
          Sam Hemelryk added a comment - Hi DS , do you mind having a quick look at this issue in case you know something about it.
          Hide
          Sam Hemelryk added a comment -

          Linking to MDL-28195 as the issue that led to this being noticed.

          Show
          Sam Hemelryk added a comment - Linking to MDL-28195 as the issue that led to this being noticed.
          Hide
          Sam Hemelryk added a comment -

          Thanks DS, looks good to me!
          Don't forget to include testing instruction, browsing to a couple of places that use file picker and testing it still works should be enough.
          Also we need to test for regressions, certainly I think that MDL-28285 is going to be a regression, and perhaps just a quick search of JS for other strings that were being loaded by this would be a good idea.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks DS, looks good to me! Don't forget to include testing instruction, browsing to a couple of places that use file picker and testing it still works should be enough. Also we need to test for regressions, certainly I think that MDL-28285 is going to be a regression, and perhaps just a quick search of JS for other strings that were being loaded by this would be a good idea. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Thanks Dongsheng, this has been integrated now.

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

          Test Failed.

          "M.core_filepicker is undefined" error occurs on:
          1. Adding label and page resources to the course
          2. Editing section summary

          Show
          Rossiani Wijaya added a comment - Test Failed. "M.core_filepicker is undefined" error occurs on: 1. Adding label and page resources to the course 2. Editing section summary
          Hide
          Sam Hemelryk added a comment -

          Ahh thanks for spotting that Rosie, indeed a quick test confirms it is a problem.
          DS this has now been reverted please can you have a look at it again.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Ahh thanks for spotting that Rosie, indeed a quick test confirms it is a problem. DS this has now been reverted please can you have a look at it again. Cheers Sam
          Hide
          Dongsheng Cai added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.
          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Dongsheng Cai added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
          Hide
          Marina Glancy added a comment -

          Thanks for reporting this.

          I'm closing this issue because I believe it affects only unsupported versions of Moodle. This issue will remain here in case other users have the same problem.

          If you haven't already done so, I encourage you to upgrade to a supported version.

          Show
          Marina Glancy added a comment - Thanks for reporting this. I'm closing this issue because I believe it affects only unsupported versions of Moodle. This issue will remain here in case other users have the same problem. If you haven't already done so, I encourage you to upgrade to a supported version.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: