_image_check_settings() is awful.
For starters, it can get you trapped in a loop of evil: #166145: Images directory declared "The directory does not exist." but changing image_default_path in variable table works; more generally it's a hangover from 4.whatever days.

Currently it does two things:
1. creates directories if necessary
2. Checks for the image toolkit.

Both of these checks are also in hook_requirements.

Is that enough for the toolkit check?

When should the directories be created? What do we do about users who create an image node before they've gone to set the image path? (what does core do for the main files directory?)

We also need to fix the problem of subfolders, until we get to D7 and #515280: file_check_directory() should create recursively works for us.

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

It seems that at least 50% of all invocations live in administrative functions.

Neither Drupal core, nor any other module usually ensures/verifies proper settings at runtime. I'm not sure whether that image_get_toolkit() check is actually invoked at any time (does core really allow to (un)configure none?).

As mentioned in #166145-24: Images directory declared "The directory does not exist." but changing image_default_path in variable table works:

Obviously, the configuration should be verified in the form validation handler.

So, I'm not entirely sure whether the invocations from image_attach/image_gallery are really required, but if they are not, then we can simply remove all direct invocations and add

$form['#validate'] = array('_image_check_settings');

in image module's settings form.

Also, we remove that image_get_toolkit() check, because I think that is outside of Image module's responsibility.

joachim’s picture

joachim commented

What concerns me here was that unlike filefield where the user HAS to specify a file location before they create content, users can enable the module, make nodes, THEN decide to set up their image directory.

So what happens if files/images has not yet been created and the user tries to create a node?
Should we create this directory on module install?

joachim’s picture

joachim commented
Status: Active » Postponed

Postponing this -- what needs to be fixed ist JUST #166145: Images directory declared "The directory does not exist." but changing image_default_path in variable table works -- we can come back and tidy this later.