Fully process local CSS file paths

Patch solved it for me! Thanks! I got to this issue from #1547918: Page not found in log (public://) #1485444: Notice: page not found - public:/css_injector/css_injector_1.css.

Setting correct status.

The patch looks fine other than coding style (spacing) issues:

• file_uri_scheme($filepath)==='public'
  

  should be

  file_uri_scheme($filepath) === 'public'
  

• $files[]=file_create_url($filepath);
  

  should be

  $files[] = file_create_url($filepath);
  

Also, I would think that the identity operator (===) for the URI scheme isn't really necessary. Equality (==) will do, but that's just my observation of core style.

Looks good.

Regarding my point in #4, I believe that public should be the major (if not the only) scenario apart from usual module/theme asset paths. When alternative stream wrappers (other than public, private, and temporary, of which only public should be used for assets) for drupal_add_css() really become a major use case I suppose a new issue should be opened to revisit this problem.

Thanks for moving this along @zhangtaihao!

Actually, is there any reason for why we're not using file_create_url($filepath) in both cases? file_create_url() should be able to handle both. Did you try whether we can simply replace the current line?

We should also make sure that the fully-qualified URI (including base URL; i.e., protocol, domain) is properly processed later on. I'm not sure whether there was a particular reason for just using base_path() previously. Did you investigate that?

Lastly, there's still trailing white-space in this patch.

Hmm... That's a good point. I'll have a look into the history of those lines in a bit.

Sorry I've been busy working on another project (which, incidentally, has also to do with file paths).

I have just gone through drupal_pre_render_styles() just to see how core renders style links from drupal_add_css(). The following are some observations:

• CSS paths with type 'external' are used verbatim.
• CSS paths with type 'file' can just be passed through to file_create_url().
• As a principle, CSS paths are not checked with file_exists() in order to minimize load on the file system. The existence of a CSS file is the responsibility of whichever module that added it.

The only possible concerns are aggregation (i.e. 'preprocess'), browser targeting, and mixed-media style sheets, but we'll shelve those for now.

As such, this issue isn't even technically about public streams, but file paths in general, which should all be put through file_create_url() to get URLs. Given wysiwyg_get_css() is supposed to "Retrieve stylesheets for HTML/IFRAME-based editors", I suppose that's what we'll do. We'll also ignore checking whether resulting URL is empty, since adding a style sheet that doesn't have an external URL is daft anyway.

I've attached a patch. It's minimal, but it should do the job.

I think the current behavior is a result of file_create_url() doing something very different in Drupal 6. Back then it would rewrite all urls differently based on public/private files being in use. The Drupal 7 version of file_create_url() won't forcibly decide the "type" of file (private/public) since they now use different schemes and can coexist.

Thus, I think this should be applied to the D7 version, but not get backported to D6. Agree?

I believe so. In any case, public://css_injector/css_injector_1.css wouldn't occur in Drupal 6, so I think this is purely an issue triggered by stream wrappers in D7.

Since external paths are accepted by file_create_url() as well, why not pass everything but 'inline' through it too? That would allow altering all the paths if needed.

Still, there are quite a lot of places where we just use base_path(), mostly for plugin files.

While I would personally have targeted 'file' and 'external' specifically, #14 is perfectly fine, assuming drupal_add_css() is intended for files that can be passed through file_create_url() and that 'inline' is the only foreseeable exception.

As for other occurrences of base_path(), I would argue that, for now, they are used correctly in plugin files since it is the most efficient and system-agnostic method of passing assets through (this assessment may change in Drupal 8 with Assetic).

Verified that #14 works: https://drupal.org/node/1793704#comment-7205846

Dear TwoD, could you commit this please? :)

Oki, done!

The 7.x-2.x-dev snapshot will be updated within 12hrs and this will be part of the next Wysiwyg 7.x-2.x release.
I've not backported it to 6.x-2.x since there is no hook_file_url_alter() in D6.