After uploading custom logo for theme the path points to root

I found out that first of all it have not set the "file_public_path" (which set the current files directory in the variable table) when you install a clean Drupal 7 installation but most important of all the form where you upload the custom logo should return the filepath relative to default files folder instead of just the filename in the field where your logos filename is shown.

This patch solves this by prepend the "default files path" to the uploaded logo when it prints it to the custom logo field in the form.

changed >>> 7.x-dev

and changed >>> system.module

Renamed my first core 7 patch, now I'm trying to follow the naming convention.
Sorry for that!
/ BoboDrone

sub

I think this bug is pretty serious since all logos will disappear the second time you post the theme settings form of any reason (if not have set file_public_path before manually)

Therefore I change this to major.

/ Bobodrone

The Drupal 7 release party in Gothenburg by NodeOne will update, test and review this patch so we hopefully can mark this as RTBC tonight.

Hi!

The patch looks fine apart from that the trailing slash should not be in the default value, this way, the file public path is the same as in other places in core.

I have tried the patch by fabsor and it works.

Oups, forgot to change status :/

Small offset. Re-rolled patch. Still works perfectly.

I would really love to see us add a test for this so it doesn't end up happening again.

Here is a test for that.

Confirmed system-admin-custom-logo-901724-12.patch fixes issue.

Confirmed 901724-14.patch(test case only) correctly detects issue before system-admin-custom-logo-901724-12.patch is applied and Passes successfully with system-admin-custom-logo-901724-12.patch applied.

Test seems to be sufficient for detecting this issue.

Weird. I can't make the test fail on my machine. Try again, bot!

#14: 901724-14.patch queued for re-testing.

Tests looks fine to me, and I can't reproduce this locally either. I will try to investigate further.

#14: 901724-14.patch queued for re-testing.

#14: 901724-14.patch queued for re-testing.

+++ b/modules/system/system.admin.inc
@@ -465,7 +465,7 @@ function system_theme_settings($form, &$form_state, $key = '') {
-      $logo_path = file_uri_target($logo_path);
+      $logo_path = variable_get('file_public_path', conf_path() . '/files') . '/' . file_uri_target($logo_path);

Seriously, don't we have an existing helper function for this...?

I really hope so, but I'm not very familiar with D7's File API yet.

Powered by Dreditor.

Related: #1087250: Custom logo and favicon stored in private filesystem if it is the default

+      $logo_path = variable_get('file_public_path', conf_path() . '/files') . '/' . file_uri_target($logo_path);

Seriously, don't we have an existing helper function for this...?

How about $logo_path = drupal_realpath('public://') . '/' . file_uri_target($logo_path);

#1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths will likely make this issue obsolete.

#1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths was committed to 8.x, and is in the process of being backported to 7.x.

I'm uploading just naxoc's test for this issue by itself, if that passes we can either combine the tests with the other issue, or if similar coverage was added elsewhere, just close this one.

Closing. There is a test in #1376166: Custom logo and favicon functionality inanely tries to support absolute local file paths now.