Need DRUPAL_ROOT in various places [#1924236]

I have an external PHP script that bootstraps Drupal, and that lives in a subfolder of my Drupal install dir.

After bootstrapping, this script calls some functionality from the Wysiwyg module. I discovered that the functionality doesn't currently work if called from a script that lives outside of the Drupal root, as there are several places where it should be (but is not) prepending DRUPAL_ROOT to the file path.

Attached patch fixes this, by appending DRUPAL_ROOT where necessary.

This also depends on: #1924212: Need DRUPAL_ROOT in file_scan_directory

Comment	File	Size	Author
#2	wysiwyg_drupalroot_moduleroot.patch	1.46 KB	Jaza
	wysiwyg_drupalroot.patch	1.73 KB	Jaza

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

TwoD

Swedish

Sweden

CreditAttribution: TwoD commented 22 February 2013 at 16:07

It seems we would need to use DRUPAL_ROOT in a lot more places for this to work with all editors (and a lot of other modules, even core). Why not just cd to the Drupal root dir in the script first?

Btw, your patch was not created from the Wysiwyg root so it needs to be applied with the -p6 switch.

Comment #2

Jaza CreditAttribution: Jaza commented 22 February 2013 at 21:03

File	Size
wysiwyg_drupalroot_moduleroot.patch	1.46 KB

@TwoD: Why not just cd to the Drupal root dir in the script first? Because 'cd'ing to the Drupal root dir in the script would be a hack. If not all Drupal code properly uses DRUPAL_ROOT, that is a bug. I'm contributing patches to instances of this bug where I find them, rather than hiding the bug with hacks.

In theory, you should be able to bootstrap Drupal from a script that's not in the root dir, and then execute whatever functionality you want from within installed Drupal modules. In my experience so far, this theory actually works in practice with very few bugs, and the the bugs that I've found, I've reported and provided patches for.

New patch attached with paths relative to Wysiwyg module root.

Comment #3

sun

German

Karlsruhe

CreditAttribution: sun commented 23 February 2013 at 01:18

Category:

bug

» task

Two complementary points here:

It is probably true that those filesystem paths should be rooted. But as @TwoD mentioned, we'd have to patch our entire code base accordingly. Can you clarify whether you checked for all instances, or did you check for your particular use-case of TinyMCE only?
If you are using a custom script to boot Drupal, which does not happen to live in Drupal's root directory, then it is the responsibility of your custom script to chdir() to Drupal's root directory. All of the custom/complementary scripts in Drupal core are doing that, and so should yours. I'm actually very surprised that your ride into bugs starts in wysiwyg.module. I'm relatively confident that I can guarantee you that your ride won't end here. What you're trying to do is known to not work in D8 even, despite a massive move towards OO-namespaced code and a class loader that looks up files based on the Drupal root.

Both things combined, I consider this a task; and definitely not a bug. Given some more clarification on 1) (and perhaps a revised patch to cater for all instances?), we can happily adjust Wysiwyg's code-base, but all things combined, I doubt that it's of any help.

Comment #4

TwoD

Swedish

Sweden

CreditAttribution: TwoD commented 23 February 2013 at 14:32

@Jaza, ok, I see. Just wanted to know if it was a matter of principles or actual technical problems with changing the working directory first. I agree that in an ideal situation it would be practical if Drupal could be bootstrapped from anywhere, but like sun said, it's difficult.

I think this just takes care of TinyMCE since at least CKEditor also has an almost identical line. I bet at least all the version detection callbacks do, and probably others too. I see you assigned this to yourself @Jaza, need a hand hunting down all instances or are you already on it?

Comment #5

Jaza CreditAttribution: Jaza commented 26 February 2013 at 13:01

Assigned:	Jaza	» Unassigned
Status:	Needs review	» Needs work

@sun:

1. No, I did not check for all instances of this bug, I only patched to the point that I got it working for my use case (and honestly, doing that was more than enough work, and very tedious debugging work it was too). No, sorry, I am not volunteering to hunt down all instances of this bug, as I said I'm just contributing patches to instances of the bug where I find them, I'm not looking for more problems than those that come my way.

2. Thanks for letting me know that the "official Drupal line" is actually that custom scripts should chdir to the Drupal root dir, I did not know that, and I did not know that this is what the complementary scripts in core do (and I just checked, it is indeed what they do). My ride into bugs did not start in Wysiwyg, it started in core (and then in some other contrib modules before Wysiwyg) - however I encountered surprisingly few bugs in core and in other contrib modules.

I don't think that this thread is particularly the place to continue debating the merits / difficulties / practicalities of bootstrapping Drupal outside of the root dir, as all that is (of course) not related to the Wysiwyg module. These are the issues I've submitted for this bug for core:

- #1891708: Need DRUPAL_ROOT in include of template.php
- #1891802: Need DRUPAL_ROOT when rendering CSS include links
- #1924212: Need DRUPAL_ROOT in file_scan_directory

If this discussion should be continued somewhere, it should be in one of the above threads (or in a new thread also filed under core). Ironic that more discussion has taken place - and more information has come to light - here in a contrib thread; but hey, that's life.