Moodle

SMARTPIX FAILS

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9, 1.9.1, 1.9.2
  • Fix Version/s: 1.9.5
  • Component/s: Themes
  • Labels:
    None
  • Environment:
    Windows Server 2003 Standard Edition (64 and 32 BIT)
    Moodle versions 1.9.x
    MySQL 5 Community Server Edition
    PHP 5.2.5

Description

When activating smartpix several logs and pics are no longer availiable to plugins such as Feedback

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andreas Grabs added a comment -

Hi,
sorry, I can't reproduce this behavior. I tested on linux (php 5.2.6) and windows Vista (php 5.2.5) both with moodle 1.9.1 and 1.9.2. All looks fine.
I used the theme chameleon. If the "smartpix"-option was unchecked so no feedback-icons were shown. If the option was checked all icons were shown as expected.
Andreas

Show
Andreas Grabs added a comment - Hi, sorry, I can't reproduce this behavior. I tested on linux (php 5.2.6) and windows Vista (php 5.2.5) both with moodle 1.9.1 and 1.9.2. All looks fine. I used the theme chameleon. If the "smartpix"-option was unchecked so no feedback-icons were shown. If the option was checked all icons were shown as expected. Andreas
Hide
Permalink
Rhodry Korb added a comment -

I am going to provide a link demonstrating this issue later today

Show
Rhodry Korb added a comment - I am going to provide a link demonstrating this issue later today
Hide
Permalink
Andreas Grabs added a comment -

I'm sorry, I don't come into your course. The login is ok but the enrolment key seems to be wrong.

Show
Andreas Grabs added a comment - I'm sorry, I don't come into your course. The login is ok but the enrolment key seems to be wrong.
Hide
Permalink
Andreas Grabs added a comment -

Hi,

in your course is no feedback-activity. What do I wrong?

Andreas

Show
Andreas Grabs added a comment - Hi, in your course is no feedback-activity. What do I wrong? Andreas
Hide
Permalink
Tim Hunt added a comment -

sam, have you seen this bug?

Show
Tim Hunt added a comment - sam, have you seen this bug?
Hide
Permalink
Sam Marshall added a comment -

No, I haven't seen this bug before.

The test site given is no longer available (understandably, since that was six months ago). If somebody wishes us to investigate, I'd like to see the following:

  • the URL within moodle (eg. /mod/resource/view.php) of the page that includes the icon that doesn't appear when smartpix is on
  • the full URL it is trying to load for the icon that doesn't load when smartpix is on (you can get this from image properties on the broken image, or in the html source where the image ought to be)

Otherwise the bug should probably be closed...

Show
Sam Marshall added a comment - No, I haven't seen this bug before. The test site given is no longer available (understandably, since that was six months ago). If somebody wishes us to investigate, I'd like to see the following:
  • the URL within moodle (eg. /mod/resource/view.php) of the page that includes the icon that doesn't appear when smartpix is on
  • the full URL it is trying to load for the icon that doesn't load when smartpix is on (you can get this from image properties on the broken image, or in the html source where the image ought to be)
Otherwise the bug should probably be closed...
Hide
Permalink
Mauno Korpelainen added a comment -

Tim and Sam - don't close this yet...

In http://moodle.org/mod/forum/discuss.php?d=89094 Shane Elliott noticed that Smartpix fails if theme names have UPPER CASE LETTERS. Otherwise Smartpix seems to work ok.

Show
Mauno Korpelainen added a comment - Tim and Sam - don't close this yet... In http://moodle.org/mod/forum/discuss.php?d=89094 Shane Elliott noticed that Smartpix fails if theme names have UPPER CASE LETTERS. Otherwise Smartpix seems to work ok.
Hide
Permalink
Tim Lock added a comment -

Here is a patch to fix the issue that affects the latest Stable Moodle - 1.9.4+ (Build: 20090303).

Summary for the fix is related to the preg match on line 114 :-

if(!preg_match('|^/([a-z0-9_\-.]+)/([a-z0-9/_\-.]+)$|',$path,$match)) {

TO

if(!preg_match('|^/([a-zA-Z0-9_\-.]+)/([a-zA-Z0-9/_\-.]+)$|',$path,$match)) {

Show
Tim Lock added a comment - Here is a patch to fix the issue that affects the latest Stable Moodle - 1.9.4+ (Build: 20090303). Summary for the fix is related to the preg match on line 114 :- if(!preg_match('|^/([a-z0-9_\-.]+)/([a-z0-9/_\-.]+)$|',$path,$match)) { TO if(!preg_match('|^/([a-zA-Z0-9_\-.]+)/([a-zA-Z0-9/_\-.]+)$|',$path,$match)) {
Hide
Permalink
Sam Marshall added a comment -

OK I have applied the patch.

I don't think we should be allowing upper-case names for themes (or for image URLs), however I also don't think smartpix.php is the place to start enforcing that. So I went ahead and applied the patch. thanks!

Show
Sam Marshall added a comment - OK I have applied the patch. I don't think we should be allowing upper-case names for themes (or for image URLs), however I also don't think smartpix.php is the place to start enforcing that. So I went ahead and applied the patch. thanks!
Hide
Permalink
Tim Lock added a comment -

Hi Sam,

Thanks for including it in the next release. Smart Pix helps us with theme icon set issues with customised themes.

Show
Tim Lock added a comment - Hi Sam, Thanks for including it in the next release. Smart Pix helps us with theme icon set issues with customised themes.
Hide
Permalink
Tim Hunt added a comment -

I agree, plugin names (including theme names) should only every comprise lower case letters. If you do anything else, you are asking for trouble.

Having said that, the closest PARAM type we have in lib/moodlelib.php is PARAM_SAFEDIR = PARAM_ALPHANUMEXT, which does allow uppercase letters. so this fix is correct.

Show
Tim Hunt added a comment - I agree, plugin names (including theme names) should only every comprise lower case letters. If you do anything else, you are asking for trouble. Having said that, the closest PARAM type we have in lib/moodlelib.php is PARAM_SAFEDIR = PARAM_ALPHANUMEXT, which does allow uppercase letters. so this fix is correct.

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.