Moodle
  1. Moodle
  2. MDL-27676

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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
    • Rank:
      17340

      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.

        Issue Links

          Activity

          Hide
          moodle.com added a comment -

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

          Show
          moodle.com added a comment - check if there is a small default picture in Moodle.. not sure
          Hide
          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
          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
          Sam Hemelryk added a comment -

          I should add I tested quickly and it worked fine.

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

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

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

          sorry the other Sam

          Show
          Jérôme Mouneyrac added a comment - - edited sorry the other Sam
          Hide
          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
          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
          Rossiani Wijaya added a comment -

          Hi Sam,

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

          Thanks

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

          Has sense to backport this to 20_STABLE?

          Show
          Eloy Lafuente (stronk7) added a comment - Has sense to backport this to 20_STABLE?
          Hide
          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
          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
          Sam Hemelryk added a comment -

          All ready for you Eloy thanks

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

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          This is now upstream, yay! Many thanks!

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

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

          Show
          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: