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

          Attachments

            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