Revealed via #1373142: Use the Testing profile. Speed up testbot by 50%

Problem

  • The theme settings form allows to configure an absolute local filesystem path for the site logo and favicon, but such a path cannot be resolved into a web-accessible URL afterwards and thus can't be supported.
  • The related tests manifest the bug by explicitly asserting that an entirely bogus URL is output for the logo.

Goal

  • Fix this utter mess.

Details

  • Currently, the theme settings form supports stream wrapper URIs (public://mylogo.png), relative paths within the public file system (mylogo.png), and lastly (but this is bogus) absolute paths to local files (/var/www/example.com/mylogo.png).
  • While absolute paths to local files are reported as valid, Drupal is not able to reliably figure out the web-accessible URL to such a file. Because this is actually unsupported in every way, you get what you deserve:
    http://example.com/drupal/%2Fvar%2Fwww%2Fexample.com%2Fsites%2Fdefault%2Ffiles%2Fmylogo.png

    However, you can successfully submit the theme settings form.

    Thus, absolute paths never worked. And support for that can be safely dropped.

  • The functional test for theme settings actually verifies that aforementioned, utterly bogus URL is output in the theme. (Victory!...)
  • Users are highly confused about what kind of path can actually be entered for the custom logo and favicon, because pretty much everything that isn't a stream wrapper URI is rejected as invalid.

User interface changes

  • The fix committed to D8 includes the following string changes:
    • Several descriptions removed
    • "Use the default logo" changed to "Use the default logo supplied by the theme"
    • "Use the default shortcut icon" changed to "Use the default shortcut icon supplied by the theme"
    • Automatically generated example paths added.
  • The current backport in #43 removes these string changes.
  • We need to decide if the string changes are backportable.

Before patch

logo_before.png

After patch


Related issues

#1209314: "Path to custom logo" needs to clarify and give some examples
#901724: After uploading custom logo for theme the path points to root
#924396: _system_theme_settings_validate_path() fails on PHP 5.2
#1087250: Custom logo and favicon stored in private filesystem if it is the default

Original report:

SystemThemeFunctionalTest::testThemeSettings() submits a drupal_realpath() to configure the site logo in theme settings. That should not and cannot work, but does, for some unknown reason, with the Standard profile.

Files: 
CommentFileSizeAuthor
#62 drupal-1376166-62.patch6.11 KBsheise
PASSED: [[SimpleTest]]: [MySQL] 39,114 pass(es).
[ View ]
#55 drupal-1376166-55.patch5.91 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 39,015 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#50 logo_before.png38.61 KBxjm
#43 interdiff.txt5.97 KBtim.plunkett
#43 drupal-1376166-43.patch8.37 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 38,765 pass(es).
[ View ]
#43 drupal-1376166-43-test-only.patch4.83 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 38,760 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#41 1376166-d7-41.patch12.2 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 37,967 pass(es), 23 fail(s), and 17 exception(s).
[ View ]
#39 1376166-d7-backport.patch12.09 KBxjm
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in modules/system/system.admin.inc.
[ View ]
#31 drupal8.system-theme-settings.31.patch14.35 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,348 pass(es).
[ View ]
#28 drupal8.system-theme-settings.28.patch14.34 KBsun
FAILED: [[SimpleTest]]: [MySQL] 34,504 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#28 system-theme-settings.28.png7.56 KBsun
#24 drupal8.system-theme-settings.24.patch16.67 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,487 pass(es).
[ View ]
#16 drupal8.system-theme-settings.16.test-only.patch9.92 KBsun
FAILED: [[SimpleTest]]: [MySQL] 34,480 pass(es), 20 fail(s), and 15 exception(es).
[ View ]
#16 drupal8.system-theme-settings.16.patch17.56 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,498 pass(es).
[ View ]
#15 1376166-custom-logo.png59.55 KBagentrickard
#14 broken.png69.48 KBdroplet
#13 system-theme-settings.12.png26.54 KBsun
#12 drupal8.system-theme-settings.12.patch13.19 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,436 pass(es).
[ View ]
#11 drupal8.system-theme-settings.11.patch13.17 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,420 pass(es).
[ View ]
#10 drupal8.system-theme-settings.10.patch13.01 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,426 pass(es).
[ View ]
#9 drupal8.system-theme-settings.9.patch11.21 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,426 pass(es).
[ View ]
#7 drupal8.system-theme-settings.6.patch11.26 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,440 pass(es).
[ View ]
#5 drupal8.system-theme-settings.5.patch4.87 KBsun
FAILED: [[SimpleTest]]: [MySQL] 34,359 pass(es), 5 fail(s), and 0 exception(es).
[ View ]
#2 drupal8.system-theme-settings.2.patch3.66 KBsun
FAILED: [[SimpleTest]]: [MySQL] 34,336 pass(es), 2 fail(s), and 0 exception(es).
[ View ]
drupal8.system-theme-test.0.patch664 bytessun
PASSED: [[SimpleTest]]: [MySQL] 34,235 pass(es).
[ View ]

Comments

sun’s picture

Status:Needs review» Active

Witness:

1) http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/core/modul...

2) http://api.drupal.org/api/drupal/includes--file.inc/function/file_uri_ta...

This means you can actually submit a full local absolute file path there.

In turn, this pretty much circles back into #1376122: Stream wrappers of parent site are leaking into all tests:

public:// is a different location in the filesystem when properly initializing the testing environment.

Since it is not properly initialized, the file path that is submitted cannot be found, because public:// is translated to a different location in the parent site vs. the child site.

sun’s picture

Status:Active» Needs review
StatusFileSize
new3.66 KB
FAILED: [[SimpleTest]]: [MySQL] 34,336 pass(es), 2 fail(s), and 0 exception(es).
[ View ]

Attached patch proves that the test fails and that there is a functional bug.

Status:Needs review» Needs work

The last submitted patch, drupal8.system-theme-settings.2.patch, failed testing.

sun’s picture

Issue tags:+Testing system

So this is actually a pure functional bug.

Users may input an absolute local file path to the logo, and that is even explicitly supported by the form validation.

However, when $logo is prepared for theming, the value is directly passed into file_create_url(), without checking whether it is an absolute local file path, so this results in an image 'src' attribute of:

http://example.com/drupal/%2Fvar%2Fwww%2Fexample.com%2Fsites%2Fdefault%2Ffiles%2Fmylogo.png

Witness user confusion: #1209314: "Path to custom logo" needs to clarify and give some examples ;)

And of course, more:
#901724: After uploading custom logo for theme the path points to root
#924396: _system_theme_settings_validate_path() fails on PHP 5.2

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new4.87 KB
FAILED: [[SimpleTest]]: [MySQL] 34,359 pass(es), 5 fail(s), and 0 exception(es).
[ View ]

This is totally busted.

Status:Needs review» Needs work

The last submitted patch, drupal8.system-theme-settings.5.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new11.26 KB
PASSED: [[SimpleTest]]: [MySQL] 34,440 pass(es).
[ View ]

Fixed pretty much everything.

Not only the tests are bogus, the entire functionality is bogus.

Horrible.

sun’s picture

Title:SystemThemeFunctionalTest attempts to submit an absolute (real) local file path for the site logo» Custom logo and favicon functionality is completely broken
Issue tags:+DrupalWTF
sun’s picture

Issue summary:View changes

Updated issue summary.

sun’s picture

Issue summary:View changes

Updated issue summary.

sun’s picture

StatusFileSize
new11.21 KB
PASSED: [[SimpleTest]]: [MySQL] 34,426 pass(es).
[ View ]

Fixed the value for the @implicit-public-file example in the description.

sun’s picture

StatusFileSize
new13.01 KB
PASSED: [[SimpleTest]]: [MySQL] 34,426 pass(es).
[ View ]

Harmonized the #description for both form elements.

sun’s picture

StatusFileSize
new13.17 KB
PASSED: [[SimpleTest]]: [MySQL] 34,420 pass(es).
[ View ]

Usage of base_path() for the local file paths was bogus; must always be semi-absolute; i.e., absolute within the document root.

sun’s picture

StatusFileSize
new13.19 KB
PASSED: [[SimpleTest]]: [MySQL] 34,436 pass(es).
[ View ]

$friendly_path needs to be reset to NULL due to the foreach loop.

sun’s picture

StatusFileSize
new26.54 KB

FWIW, this is how the form looks:

system-theme-settings.12.png

droplet’s picture

Status:Needs review» Needs work
StatusFileSize
new69.48 KB

Thanks. ref from #1209314: "Path to custom logo" needs to clarify and give some examples

previews and example URL broken

agentrickard’s picture

StatusFileSize
new59.55 KB

Relative paths work for for me in both D7 and D8. I don't understand this part of the OP:

[EDIT: Redacted after re-reading queue.] This still needs a UX fix at least.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new17.56 KB
PASSED: [[SimpleTest]]: [MySQL] 34,498 pass(es).
[ View ]
new9.92 KB
FAILED: [[SimpleTest]]: [MySQL] 34,480 pass(es), 20 fail(s), and 15 exception(es).
[ View ]

Thanks @agentrickard, that was very very very helpful! :)

In the end, it's a lil' less broken than stated here, and I've to fully admit that my revised test was hella broken, too.

sun’s picture

Issue summary:View changes

Updated issue summary.

sun’s picture

Title:Custom logo and favicon functionality is completely broken» Custom logo and favicon functionality inanely tries to support absolute local file paths

Updated the summary; also adjusting title.

The test-only patch failed as expected (most but not all failures caused by the missing additional description), the full patch passed.

This looks RTBC to me.

Except, perhaps, the change to DrupalLocalStreamWrapper, which I'm not 100% sure of. I assumed it would fix the bogus file_exists() result for a stream wrapper URI of 'public://', but that wasn't the case - which is why the form validation function explicitly checks for that and also contains a @todo for it.

droplet’s picture

Status:Needs review» Needs work

TWO issues:
First:
1. Upload an exists image

Use mama3.jpg to reference a file in the public filesystem, public://mama3.jpg for a specific filesystem, or /sites/default/files/mama3.jpg for an arbitrary local file within this site.

2. change custom path to "/sites/default/files/mama3.jpg" and SAVE

Use logo.png to reference a file in the public filesystem, /sites/default/files/mama3.jpg for a specific filesystem, or //sites/default/files/mama3.jpg for an arbitrary local file within this site.

Can we change the texts to static strings, may like this:

Use public://logo.png to reference a file in the public filesystem. "custom_path/logo.png" for an arbitrary local file relative to the Drupal installation directory.

possible make it same to admin/config/media/file-system?

2nd:
When drupal installed on Sub-dir, eg. http://example.com/drupal

"/sites/default/files/mama3.jpg" refer to "http://example.com/sites/default/files/mama3.jpg"

same @#14 errors happened.

EDIT:
doesn't it suppport '../filename.png' ??

sun’s picture

Status:Needs work» Needs review

@droplet: The examples and steps you've provided still seem to be based on the second to last patch. That patch attempted to introduce "semi-absolute" paths (starting with '/' to point to a file within Drupal's root), but this was bogus and has been removed in the latest patch.

droplet’s picture

@sun,

You right. Working now :). The latest stuff I seen is drupal will produce an ugly URL:
<img src="http://192.168.56.108/drupal8x/../test2/logo.png" alt="Home" />

sun’s picture

Thanks for confirming!

I'm not sure whether we need to care for a path like ../logo.png - I don't think that's really supported; though I can imagine the theme settings form allows you to submit such a value.

sun’s picture

Any other feedback/reviews? This looks good to go for me.

sun’s picture

Status:Needs review» Needs work
+++ b/core/includes/stream_wrappers.inc
@@ -301,6 +301,10 @@ abstract class DrupalLocalStreamWrapper implements DrupalStreamWrapperInterface
+    // A target derived of a URI "scheme://" is invalid.
+    if ($target === '') {
+      return FALSE;
+    }

I'm really not sure about this, so I'm going to back it out of this patch.

+++ b/core/modules/system/system.admin.inc
@@ -657,13 +674,24 @@ function system_theme_settings_validate($form, &$form_state) {
+  // @todo file_exists() returns TRUE when $path is "public://".
+  if (file_uri_target($path) === '') {
+    return FALSE;
+  }
...
+  if (file_exists($path)) {

Actually, a simple change to is_file() should yield the proper result here.

As you know, file_exists() does not care for whether the file path is a directory or file.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new16.67 KB
PASSED: [[SimpleTest]]: [MySQL] 34,487 pass(es).
[ View ]

Excellent.

xjm’s picture

+++ b/core/modules/simpletest/drupal_web_test_case.phpundefined
@@ -3138,8 +3138,21 @@ class DrupalWebTestCase extends DrupalTestCase {
-  protected function assertFieldByName($name, $value = '', $message = '') {
-    return $this->assertFieldByXPath($this->constructFieldXpath('name', $name), $value, $message ? $message : t('Found field by name @name', array('@name' => $name)), t('Browser'));
+  protected function assertFieldByName($name, $value = NULL, $message = NULL) {
+    if (!isset($message)) {
+      if (!isset($value)) {
+        $message = t('Found field with name @name', array(
+          '@name' => var_export($name, TRUE),
+        ));
+      }
+      else {
+        $message = t('Found field with name @name and value @value', array(
+          '@name' => var_export($name, TRUE),
+          '@value' => var_export($value, TRUE),
+        ));
+      }
+    }
+    return $this->assertFieldByXPath($this->constructFieldXpath('name', $name), $value, $message, t('Browser'));

I asked sun about this bit. This change seems somewhat out of scope to me. The assertion is used once in testThemeSettings(). It's a fairly simple cleanup and I'm not opposed to it being added here or anything, but I thought I'd point it out.

Changing the theme tests from standard/bartik to testing/stark also isn't part of the scope in the title, but sun explained that it's what exposes the bug and makes it testable.

xjm’s picture

+++ b/core/modules/system/system.admin.incundefined
@@ -530,6 +513,40 @@ function system_theme_settings($form, &$form_state, $key = '') {
+      $element['#description'] = t('Use <code>@implicit-public-file</ code> to reference a file in the public filesystem, <code>@explicit-file</ code> for a specific filesystem, or <code>@local-file</ code> for an arbitrary local file within this site.', array(

Hmmm, "An arbitrary local file within the site" seems pretty confusing. What does it mean?

+++ b/core/modules/system/system.admin.incundefined
@@ -672,6 +696,11 @@ function _system_theme_settings_validate_path($path) {
+  // Exclude unnecessary elements before saving.
+  form_state_values_clean($form_state);
+  $key = $form_state['values']['var'];
+  unset($form_state['values']['var']);
+
   $values = $form_state['values'];

   // If the user uploaded a new logo or favicon, save it to a permanent location
@@ -703,10 +732,7 @@ function system_theme_settings_submit($form, &$form_state) {

@@ -703,10 +732,7 @@ function system_theme_settings_submit($form, &$form_state) {
   if (empty($values['default_favicon']) && !empty($values['favicon_path'])) {
     $values['favicon_mimetype'] = file_get_mimetype($values['favicon_path']);
   }
-  $key = $values['var'];

-  // Exclude unnecessary elements before saving.
-  unset($values['var'], $values['submit'], $values['reset'], $values['form_id'], $values['op'], $values['form_build_id'], $values['form_token']);

Another unrelated cleanup?

sun’s picture

sun’s picture

Assigned:Unassigned» sun
StatusFileSize
new7.56 KB
new14.34 KB
FAILED: [[SimpleTest]]: [MySQL] 34,504 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Revised the description after discussion with @xjm in IRC.

Screenshot:

system-theme-settings.28.png

Status:Needs review» Needs work

The last submitted patch, drupal8.system-theme-settings.28.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
sun’s picture

StatusFileSize
new14.35 KB
PASSED: [[SimpleTest]]: [MySQL] 34,348 pass(es).
[ View ]

Fixed the fatal error by adding Node module to the list of modules to install in setUp().

This patch really looks ready to fly for me now.

SilverBallz’s picture

sun’s picture

sun’s picture

tim.plunkett’s picture

tim.plunkett’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to me. The string changes are nice clarifications, but they'll have to come out for the D7 patch.

catch’s picture

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Yes this looks good to me too. Committed/pushed to 8.x, moving to 7.x for backport.

xjm’s picture

Assigned:sun» xjm

I'll pull out the string changes for a backport.

xjm’s picture

Assigned:xjm» Unassigned
Status:Patch (to be ported)» Needs review
StatusFileSize
new12.09 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in modules/system/system.admin.inc.
[ View ]

Removing the bits to change the #title and #description, but leaving the rest.

Status:Needs review» Needs work

The last submitted patch, 1376166-d7-backport.patch, failed testing.

xjm’s picture

Status:Needs work» Needs review
StatusFileSize
new12.2 KB
FAILED: [[SimpleTest]]: [MySQL] 37,967 pass(es), 23 fail(s), and 17 exception(s).
[ View ]

This is what happens when people don't include the final serial comma in multi-line arrays. :P

Status:Needs review» Needs work

The last submitted patch, 1376166-d7-41.patch, failed testing.

tim.plunkett’s picture

Status:Needs work» Needs review
StatusFileSize
new4.83 KB
FAILED: [[SimpleTest]]: [MySQL] 38,760 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
new8.37 KB
PASSED: [[SimpleTest]]: [MySQL] 38,765 pass(es).
[ View ]
new5.97 KB

There was a whole section of the test for the D8 strings, because they have dynamic examples.
Removing that, and also removing the 'core/' prefix on the files, made it all pass locally for me.

Also, removing the hunks about switching the default theme, since #1181776: Change theme_default variable to Stark was D8 only.

Status:Needs review» Needs work

The last submitted patch, drupal-1376166-43-test-only.patch, failed testing.

tim.plunkett’s picture

Status:Needs work» Needs review
xjm’s picture

Status:Needs review» Reviewed & tested by the community

Yeah, so that's actually a Drupal 7 patch, huh? Thanks Tim!

I was concerned that there are fewer test failures than in #16, but I double-checked and (of course) these were related to the dynamic strings (as stated in #44).

sun’s picture

Assigned:Unassigned» webchick

We need to ask @webchick whether the string changes can be backported.

IMHO, they are an essential part of the fix, since site admins don't really have a clue of what values can actually be configured. The string changes are administrative only.

sun’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

Issue summary:View changes

Updated issue summary.

xjm’s picture

I added the before/after screenshots to the summary, listed the string changes, and tried to clarify the current status of the patch.

xjm’s picture

StatusFileSize
new38.61 KB
xjm’s picture

Issue summary:View changes

Document string changes

webchick’s picture

Status:Reviewed & tested by the community» Fixed

really awesome work on this fix. i think the string breakage is acceptable and agree it's part of the fix.

committed and pushed to 7.x. thanks!

webchick’s picture

Issue tags:+7.13 release notes

also tagging for release notes

pillarsdotnet’s picture

Status:Fixed» Needs work

If the string changes are part of the fix, then we need a 7.x patch for the string changes.

xjm’s picture

Priority:Normal» Major

@pillarsdotnet is correct. Bumping to major since we'll want that followup ASAP. The task is to create a backport more like #31. It might be easiest to revert this commit and apply the complete fix after.

tim.plunkett’s picture

Status:Needs work» Needs review
StatusFileSize
new5.91 KB
FAILED: [[SimpleTest]]: [MySQL] 39,015 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Here are the changes.
I got a weird failure locally that I hope doesn't fail here. The same assertion works in D8.

Status:Needs review» Needs work

The last submitted patch, drupal-1376166-55.patch, failed testing.

tim.plunkett’s picture

Damn, those are the failures I saw locally. For some reason path_to_theme() is picking up 'seven' and not 'stark' in D7 but not in D8.

webchick’s picture

Should I roll this back?

webchick’s picture

Priority:Major» Normal

Ok, I'm up to speed now. The patch I committed was the stripped down version, without the string fixes to make it more palatable for backport.

Since the revised patch adds / changes a lot of strings, I'd prefer to hold it to the beginning of next release to give translators a bit more time to catch up.

And since it's just string fixes, I think we can degrade to normal, too. But happy to discuss it more. I'd really prefer not to change the code base at this point if at all possible, in order to give folks sufficient time to test a non-moving target.

tim.plunkett’s picture

I would agree with this not being major. As sun explained, the string changes are part of the fix in that they convey that a fix happened at all. But it can definitely wait til after 7.13.

sun’s picture

Thanks for the commit.

The interdiff in #43 contained some more changes/reverts for D7, which we should try to incorporate in the follow-up patch.

That will also resolve the test failures, since the Standard profile sets up Bartik/Seven as default/admin themes and the test attempts to assert proper logo/favicon path examples.

Thus, we stripped away too much from the backport and should re-inject that code. Ideally, the test code in D7 should look identical to D8.

Only affects the test and string changes. The bug fix and functionality seems to be identical.

sheise’s picture

Status:Needs work» Needs review
StatusFileSize
new6.11 KB
PASSED: [[SimpleTest]]: [MySQL] 39,114 pass(es).
[ View ]

Here's an update to the patch in #55 which is the D7 version.

There were 2 reasons for the previous fails.

1. The "human-friendly values and form element descriptions for the logo" were not getting set correctly for a relative file path.

2. In the test, the relative file paths were never getting a $local_file reset, and were therefore always looking to equal the default $local_file we had set of themes/stark/logo.png. I added a fix for this but there may be a better way to go about it than what I've done.

Also I attempted to add some of the additional changes from the D8 patch as suggested in #61 but this introduced more fails so I have not included them.

sun’s picture

Issue tags:-7.13 release notes

Can someone manually check the difference in actual resulting code between D7 + #62 and D8?

It looks correct to me, but unfortunately, the D7 backport got split in a weird way.

webchick’s picture

Issue tags:+7.13 release notes

Please don't remove tags for historic reference.

tim.plunkett’s picture

Status:Needs review» Needs work
Issue tags:-7.13 release notes
+++ b/modules/system/system.admin.incundefined
@@ -539,6 +535,26 @@ function system_theme_settings($form, &$form_state, $key = '') {
+      elseif (!empty($original_path)) {
+        $local_file = $original_path;

This conditional isn't in D8.

+++ b/modules/system/system.testundefined
@@ -1677,6 +1677,40 @@ class SystemThemeFunctionalTest extends DrupalWebTestCase {
+      // Adjust for relative path not in public filesystem.
+      else {
+        $local_file = $input;

This is also not in D8.

tim.plunkett’s picture

Status:Needs work» Needs review
Issue tags:+7.13 release notes

Restoring tag/status

webchick’s picture

Assigned:webchick» Unassigned

Also, I don't think you want this issue assigned to me anymore. :)

rocket_nova’s picture

rocket_nova’s picture

sun’s picture

Status:Needs review» Needs work

Looks like we still need to incorporate #65

cheese_monkey’s picture

Please, add to the descriptions a line noting that the file must be in place before trying to enter that path into Drupal, otherwise the test comes back with "invalid file path".

I was wrestling with my site installation trying to find a path that Drupal would accept, thinking "first I'll figure out what bizarre combination of files+sites will pass its test, THEN I'll move the file to that place on the local filesystem". Turns out that a probe for file existence is part of its test.

I suppose it's all fine and well for Drupal to try and save the user from themselves, but it would be even more useful to have explicit descriptions of what exactly we can type into those fields.

cheese_monkey’s picture

Issue summary:View changes

Updated issue summary.