Hi, I've been tracking down this bug for a couple of days now and it's come down to these few lines:

function theme_media_formatter_large_icon($variables) {
  $file = $variables['file'];
  $icon_dir = variable_get('media__icon_base_directory', file_default_scheme() . '://media-icons/') . '/' . media_variable_get('icon_set');
  $icon = file_icon_path($file, $icon_dir);
  $variables['path'] = $icon;

If media__icon_base_directory is not defined this causes $icon_dir to be set as "public://media-icons//default/"

This causes issues with something at our server level configuration, I don't know how, but the url was changing to a single slash on the image_style_deliver callback causing a MENU_ACCESS_DENIED. I feel like this should be fixed anyway so here's a patch. If anyone could shed light on how this might be happening I would greatly appreciate it :)

Files: 
CommentFileSizeAuthor
#4 2076499-media-icon-base-dir.patch3.16 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 92 pass(es).
[ View ]
#3 media-icon-base-dir-double-slash-2076499-3.patch1.33 KBDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 92 pass(es).
[ View ]
media-icon-base-dir-double-slash.patch1018 bytesacbramley
PASSED: [[SimpleTest]]: [MySQL] 92 pass(es).
[ View ]

Comments

rvallejo’s picture

Just to clarify, are you working with the dev version? I'm seeing the line differently in alpha2.

acbramley’s picture

This patch relates to the dev version yes

Devin Carlson’s picture

StatusFileSize
new1.33 KB
PASSED: [[SimpleTest]]: [MySQL] 92 pass(es).
[ View ]

This also affects the copying of icons during installation.

Dave Reid’s picture

StatusFileSize
new3.16 KB
PASSED: [[SimpleTest]]: [MySQL] 92 pass(es).
[ View ]

Here's one with an update hook to fix the incorrect directory if it was set for new installs since alpha2 since we'd want to fix those. Realized that we want to hard-code the directory to public because if people have the directory set to private, then access to the thumbnails would be broken, so it makes sense to hard-code here. Also fixes the bad exception message in media_update_7216().

Dave Reid’s picture

Status:Needs review» Fixed

Tested and committed #4 to 7.x-2.x.
http://drupalcode.org/project/media.git/commit/c182078

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

acbramley’s picture

Hmmm, would've thought the person that raised and attempted to first fix this would at least get attribution...never mind.

Dave Reid’s picture

@acbramley: Do you mind elaborating what you mean? There were three different people involved in patching this issue, and all three people were given credit in the commit message itself, which is the standard Drupal community behavior for commit attribution when there is more than one person involved. How should it have been done differently? Note the commit message itself was generated using Dreditor.

acbramley’s picture

@Dave Reid, I'm not sure the convention of this but I thought it was a general rule to give the original patcher the authorship for the commit

Dave Reid’s picture

No, that is not actually the community convention.

acbramley’s picture

Apologies, must just be my own convention :)