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

pluginfile always return the big version of the default user profile image

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.0.4
    • Component/s: Files API
    • Labels:
    • Testing Instructions:
      Hide

      Run the following code without a profil image (so the default image gets used):
      $profileimageurl = moodle_url::make_pluginfile_url(get_context_instance(CONTEXT_USER, $USER->id)->id, 'user', 'icon', NULL, '/', 'f2');
      echo "<img src=\"".$profileimageurl->out(false)."\">";

      You should see a small default image (try with 'f1' to see the difference).

      Show
      Run the following code without a profil image (so the default image gets used): $profileimageurl = moodle_url::make_pluginfile_url(get_context_instance(CONTEXT_USER, $USER->id)->id, 'user', 'icon', NULL, '/', 'f2'); echo "<img src=\"".$profileimageurl->out(false)."\">"; You should see a small default image (try with 'f1' to see the difference).
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Pull Master Branch:
      wip-MDL-27676-master

      Description

      http://jerome.moodle.local/~jerome/Moodle_HEAD/pluginfile.php?file=%2F21%2Fuser%2Ficon%2Ff2 => it should return the small version of the profile image. However it is always converted to http://jerome.moodle.local/~jerome/Moodle_HEAD/theme/image.php?theme=standard&image=u%2Ff1

      It should be converted to http://jerome.moodle.local/~jerome/Moodle_HEAD/theme/image.php?theme=standard&image=u%2Ff2

      Note that it works perfectly if the profile is not the theme default profile image (e.g. the user uploaded his own image).

      I looked very quickly at the code in pluginfile.php it's probably around there:

      } else if ($component === 'user') {
          if ($filearea === 'icon' and $context->contextlevel == CONTEXT_USER) {
              if (!empty($CFG->forcelogin) and !isloggedin()) {
                  // protect images if login required and not logged in;
                  // do not use require_login() because it is expensive and not suitable here anyway
                  redirect($OUTPUT->pix_url('u/f1'));
              }
              $filename = array_pop($args);
              if ($filename !== 'f1' and $filename !== 'f2') {
                  redirect($OUTPUT->pix_url('u/f1'));
              }
              if (!$file = $fs->get_file($context->id, 'user', 'icon', 0, '/', $filename.'/.png')) {
                  if (!$file = $fs->get_file($context->id, 'user', 'icon', 0, '/', $filename.'/.jpg')) {
                      redirect($OUTPUT->pix_url('u/f1'));
                  }
              }
       
              send_stored_file($file, 60*60*24); // enable long caching, there are many images on each page
       
          }

      This problem will break the display of every third party applications requesting a small picture by url, most likely mobile apps.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            moodle.com moodle.com added a comment -

            check if there is a small default picture in Moodle.. not sure

            Show
            moodle.com moodle.com added a comment - check if there is a small default picture in Moodle.. not sure
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Repo: git://github.com/samhemelryk/moodle.git
            Branch: wip-MDL-27676-master
            Diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27676-master

            I had a quick look at this the solution seems pretty simple... what I don't know is why it was always using f1.
            I suppose you could look into it - or you could blindly trust it and just integrate... up to whoever.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Repo: git://github.com/samhemelryk/moodle.git Branch: wip- MDL-27676 -master Diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27676-master I had a quick look at this the solution seems pretty simple... what I don't know is why it was always using f1. I suppose you could look into it - or you could blindly trust it and just integrate... up to whoever. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            I should add I tested quickly and it worked fine.

            Show
            samhemelryk Sam Hemelryk added a comment - I should add I tested quickly and it worked fine.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I would feel bad to take the credit for you, I assign to you

            Show
            jerome Jérôme Mouneyrac added a comment - I would feel bad to take the credit for you, I assign to you
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            sorry the other Sam

            Show
            jerome Jérôme Mouneyrac added a comment - - edited sorry the other Sam
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Just to explain because I could sound like I don't want to do it, in yesterday meeting I explained that I have one week and half left to finish mobile core code and mobile app pariticipant issues. So I'll peer-review this by remorse, but please do not give me more

            Show
            jerome Jérôme Mouneyrac added a comment - Just to explain because I could sound like I don't want to do it, in yesterday meeting I explained that I have one week and half left to finish mobile core code and mobile app pariticipant issues. So I'll peer-review this by remorse, but please do not give me more
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Sam,

            I reviewed this issue and it works great.
            submitting for integration.

            Thanks

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Sam, I reviewed this issue and it works great. submitting for integration. Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Has sense to backport this to 20_STABLE?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Has sense to backport this to 20_STABLE?
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Eloy,

            Yes certainly has sense to backport to 20_STABLE, I'll go find it now and do that.
            Will update the issue once done.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Eloy, Yes certainly has sense to backport to 20_STABLE, I'll go find it now and do that. Will update the issue once done. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            All ready for you Eloy thanks

            Show
            samhemelryk Sam Hemelryk added a comment - All ready for you Eloy thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi I'm passing this (URLs seem correct) but, following the instructions sticky I ended with the images not displaying at all (empty box) and clicking in the resulting (correct) URLs:

            I got this (debugging enabled):

            Coding problem: this page does not set $PAGE->context properly.
            line 343 of /lib/pagelib.php: call to debugging()
            line 1249 of /lib/pagelib.php: call to moodle_page->magic_get_context()
            line 1244 of /lib/setuplib.php: call to moodle_page->initialise_theme_and_output()
            line 308 of /pluginfile.php: call to bootstrap_renderer->__call()
            line 308 of /pluginfile.php: call to bootstrap_renderer->pix_url()

            So I guess something is borked in pluginfile? with debug enabled? For your consideration.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi I'm passing this (URLs seem correct) but, following the instructions sticky I ended with the images not displaying at all (empty box) and clicking in the resulting (correct) URLs: http://127.0.0.1/~stronk7/integration/pluginfile.php/13/user/icon/f1 http://127.0.0.1/~stronk7/integration/pluginfile.php/13/user/icon/f2 I got this (debugging enabled): Coding problem: this page does not set $PAGE->context properly. line 343 of /lib/pagelib.php: call to debugging() line 1249 of /lib/pagelib.php: call to moodle_page->magic_get_context() line 1244 of /lib/setuplib.php: call to moodle_page->initialise_theme_and_output() line 308 of /pluginfile.php: call to bootstrap_renderer->__call() line 308 of /pluginfile.php: call to bootstrap_renderer->pix_url() So I guess something is borked in pluginfile? with debug enabled? For your consideration. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is now upstream, yay! Many thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is now upstream, yay! Many thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is one ping about the comment 2 positions above. Ping!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is one ping about the comment 2 positions above. Ping!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Aug/11