Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-38284 META: Improve core JavaScript processes, etc
  3. MDL-38658

Correct fullpath for dndupload.js inclusion

          Details

          • Testing Instructions:
            Hide

            Ensure that you have cachejs enabled, and jsrev not set

            • Open dev tools to the 'Network' tab
            • Open course page and turn editing on
            • Check that the course/dndupload.js was loaded through lib/javascript.php
            • Check that dndupload works
            Show
            Ensure that you have cachejs enabled, and jsrev not set Open dev tools to the 'Network' tab Open course page and turn editing on Check that the course/dndupload.js was loaded through lib/javascript.php Check that dndupload works
          • Difficulty:
            Easy
          • Affected Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE
          • Pull from Repository:
          • Pull Master Branch:
          • Pull Master Diff URL:

            Description

            I just discovered that the defintion in course/dnduploadlib.php for inclusion of the JS code (course/dndupload.js) uses a moodle_url, so is not using lib/javascript.php and is not being sent with caching headers.

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  dobedobedoh Andrew Nicols added a comment -

                  This will break for anyone doing strange things with forcing browser caching, and force a suck of an extra 40KB on every page load using course/dndupload.js.

                  Show
                  dobedobedoh Andrew Nicols added a comment - This will break for anyone doing strange things with forcing browser caching, and force a suck of an extra 40KB on every page load using course/dndupload.js.
                  Hide
                  Permalink
                  dobedobedoh Andrew Nicols added a comment -

                  I'm not sure whether this should be backported to stable branches, my gut feeling is that it should but I'd appreciate any input.

                  If so, I'll create stable branches after peer review.

                  Show
                  dobedobedoh Andrew Nicols added a comment - I'm not sure whether this should be backported to stable branches, my gut feeling is that it should but I'd appreciate any input. If so, I'll create stable branches after peer review.
                  Hide
                  Permalink
                  dobedobedoh Andrew Nicols added a comment -

                  P.s. Petr: Sorry for giving you this, but I suspect you know more about the potential browser caching issues than anyone else.

                  Show
                  dobedobedoh Andrew Nicols added a comment - P.s. Petr: Sorry for giving you this, but I suspect you know more about the potential browser caching issues than anyone else.
                  Hide
                  Permalink
                  skodak Petr Skoda added a comment -

                  +1, moodle_url() should not be ever used for JS files like this

                  Show
                  skodak Petr Skoda added a comment - +1, moodle_url() should not be ever used for JS files like this
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Integrated (23, 24 & master), thanks!

                  Note: the 23 branch (MDL-38658-m23) was missing so I simply cherry-picked from 24.

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks! Note: the 23 branch ( MDL-38658 -m23) was missing so I simply cherry-picked from 24.
                  Hide
                  Permalink
                  phalacee Jason Fowler added a comment -

                  I must be doing something wrong, or the testing instructions aren't thorough enough to ensure it works correctly, because I am not seeing anything.

                  Show
                  phalacee Jason Fowler added a comment - I must be doing something wrong, or the testing instructions aren't thorough enough to ensure it works correctly, because I am not seeing anything.
                  Hide
                  Permalink
                  phalacee Jason Fowler added a comment -

                  Ah, figured it out... you need to turn on editing ... whoops.

                  Show
                  phalacee Jason Fowler added a comment - Ah, figured it out... you need to turn on editing ... whoops.
                  Hide
                  Permalink
                  phalacee Jason Fowler added a comment -

                  All working. Thanks Andrew.

                  Show
                  phalacee Jason Fowler added a comment - All working. Thanks Andrew.
                  Hide
                  Permalink
                  damyon Damyon Wiese added a comment -

                  Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

                  Show
                  damyon Damyon Wiese added a comment - Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

                    People

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

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        13/May/13