Problem/Motivation

In working on a new D8 theme I found that the {{ base_dir }} variable is available in page.html.twig, but not html.html.twig - I'm not sure if that is by design, or an omission, but it would be helpful to have in conjunction with {{ active_theme_path() }} (which does work in html.html.twig).

Proposed resolution

Using base paths/base directories in file URLs is hugely problematic. We have a Twig function for this: file_url().

The remaining problem then is that you don't know the location of the theme. This is what #1308152: Add stream wrappers to access extension files solves: it allows you to use theme://theme_name as a file URI, which file_url() will transform into a file URL. Using a theme:// file URI works always, regardless of the exact location of the theme. (For example: /sites/all/themes/my_theme, /sites/mysite.com/themes/my_theme, /profiles/my_profile/my_theme …)

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

jpamental created an issue. See original summary.

mikeker’s picture

Version: 8.0.0-rc2 » 8.1.x-dev
Issue tags: +Twig

base_path is added in template_prerprocess_page() (core/includes/theme.inc:1328):

  $variables['base_path']         = base_path();
  $variables['front_page']        = \Drupal::url('');
  $variables['language']          = $language_interface;

As mentioned in #2168231: Twig Functions needed in templates these and other useful system-ish variables should be available to all templates via Twig functions or a single system variable array/object (in an effort not to overwhelm the global namespace).

I don't believe there is any reason it's in the page preprocess and not the html preprocess. At this point, I don't see this getting into 8.0.0 as it could be added by contrib. but there's no reason not to add it for 8.1.0

mikeker’s picture

Just noticed #2168231: Twig Functions needed in templates was pushed to 8.1.x as well.

jpamental’s picture

Thanks @mikeker -

I've tested out moving the variable assignment but then you don't get it available in page.html.twig, so I've added it to the template_preprocess_html() at line 1262, keeping it in place at line 1328. That did the trick for using the variable in either template. I added a reference to the variable in the all of the template files I could find (core and included themes) and rolled it all up into a patch. This was done against the 8.0.x-dev branch. Should I re-roll it agains 8.1.x?

mikeker’s picture

@jpamental, Thanks for moving this issue forward! Yes, this should be rolled against 8.1.x as I don't see it making it into 8.0.x at this point. Also, the testbot will run a patch against whatever version the issue is marked as, which will cause a bunch of other problems (like the patch not applying) between 8.0.x and 8.1.x.

However, I think a better approach would be to create a Twig global for some sort of a system or drupal variable that would hold base_path as well as other system-wide variables such as isFrontPage or activeThemeName (or however we want to rename those old D7 variables). That way any template (system generated or otherwise) would be able to call system.getBasePath() or drupal.isFrontPage(). Because, who knows, you might need the base path in your field__foo-bar.html.twig template because of some random anchor tag you're building.

Kinda thinking out loud here... If we go with that approach, it would be best to post patch to #2168231: Twig Functions needed in templates as this would solve the issues raised there as well.

jpamental’s picture

@mikeker -

I think this is easier than we thought! I took a look at your approach, and as I don't think these variables need to be altered by the theme, they just need to be read: I just moved the variable assignment into that _template_preprocess_default_variables() function like so:

// Tell all templates where they are located.
$variables['directory'] = \Drupal::theme()->getActiveTheme()->getPath();
$variables['base_path'] = base_path();

I removed all the other places where that variable is assigned and it worked perfectly to make the base_path variable available in any theme template I tried (html, page, block).

Here's an updated patch (still against 8.0.x for easier testing) - if this seems to make sense I'll reroll against 8.1.x.

jpamental’s picture

That last one is totally wrong. Rolled the patch with parts of both solutions in it. This is the right one, and essentially only needs to change one line in theme.inc: moving the assignment of $variables['base_path'] from line ~1325 or so up to ~1209.

joelpittet’s picture

Category: Bug report » Feature request

This looks like a nice thing to have. Though this is not a bug so moving to feature request. It's already scoped out for 8.1.x

Just some nit picks that could be tackled:

  1. +++ b/core/includes/theme.inc
    @@ -1209,6 +1209,7 @@ function _template_preprocess_default_variables() {
    +  $variables['base_path']         = base_path();
    

    The space before the = can be removed here.

  2. +++ b/core/includes/theme.inc
    @@ -1260,6 +1261,7 @@ function template_preprocess_html(&$variables) {
     
    +
       $site_config = \Drupal::config('system.site');
    

    nit: Extra linebreak doesn't need to be there.

jpamental’s picture

@joelpittet - noted! (I saw those too late)

I'm going to pull 8.1.x and reroll the patch, which will address that and also adding in the code comments in any other template files that need it.

joelpittet’s picture

@jpamental whoops sorry don't pull 8.1.x it's pretty useless until some time, just get the latest 8.0.x. (we usually postpone these 8.1.x issues) until that branch is actually released. I'm guessing that is because of syncing branches is a pain.

jpamental’s picture

Thanks @joelpittet!

Here's a cleaned up patch that was rolled against the latest work on 8.0.x

Sagar Ramgade’s picture

Status: Active » Needs review
Cottser’s picture

Status: Needs review » Needs work

With stable theme in core we should update its HTML template's docs as well.

We might want to consider this a task as well, and it might not hurt to add a basic test.

jpamental’s picture

@Cottser - Stable doesn't have any template files! Also - I'm not sure what to do for a test, so if you can point me in the right direction I'm happy to work on it.

Cottser’s picture

The template files were just added this morning, git pull :)

jpamental’s picture

jpamental’s picture

@Cottser - any tips on how to go about setting up a test? I'm totally in the dark there.

Wim Leers’s picture

Using base_path is wrong at least 99% of the time. Use file_create_url(), which is available in Twig as file_url().

joelpittet’s picture

Status: Needs work » Needs review

I tend to agree with @Wim Leers on this, maybe "close won't fix" is a better approach here. @jpamental is there any use-case that we may be overlooking?

The last submitted patch, 6: base_path_missing_in_html_html_twig-2611246-6.patch, failed testing.

Cottser’s picture

Can that be used to point to unmanaged files, specifically files in the theme or theme's subdirectories?

Cottser’s picture

And more specifically in a path agnostic manner so that it's not tied to the location of the theme on the filesystem.

Wim Leers’s picture

#21: yes. {{ file_url('core/misc/druplicon.png') }} should work just fine. See the docs of file_create_url().

Although relative to the theme itself, that's a different matter altogether. What's a valid use case for that though? CSS and JS should be loaded via asset libraries. CSS files can reference background images and fonts using paths relative to the CSS file. So which use case is not covered by that?

joelpittet’s picture

FYI, file_create_url() is an unfortunate name and apparently we should be using that in D7 too;)

joelpittet’s picture

re #23 you may want to avoid heavy images in your CSS for performance and have them in your template instead so that it will load with the page and the browser will hold a placeholder for less jumpiness an parallelize the download.

I'd propose something like this:
<img src="{{ file_url(active_theme_path() ~ '/img/big-ass.png') }}">

Wim Leers’s picture

Another reason why theme://themename/img/big-ass.png would've been nice. #1308152: Add stream wrappers to access extension files is still moving forward, so perhaps we can get it for 8.1.

No reason to add a new function then. The patch in #1308152: Add stream wrappers to access extension files is blocked on review. Let's give it the reviews its needs so that that can be fixed? :)

Cottser’s picture

Yeah that should work. Another example use case for theme-relative would be referencing favicon/touch icon etc. from the html.html.twig template, something I see quite often.

Jeff Burnz’s picture

Well, file_url is absolute, do we always want this, base_path allows use of directory variable and therefor is relative, I like this better than calling a twig function in the template although #23/#25 are usable examples. I really want the stream wrapper :)

Wim Leers’s picture

@Jeff Burnz: You make an excellent point, one that I wanted to make as well now that #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send landed a few days ago.

Indeed, usually (99% of the time) you actually want the file URL to be relative. And that's why #1494670 also updated the Twig extension's file_url() function:

      new \Twig_SimpleFunction('file_url', function ($uri) {
        return file_url_transform_relative(file_create_url($uri));
      }),

The wrapping in file_url_transform_relative() makes sure you end up with relative file URLs. So, as of #1494670, it's more difficult to do the thing you shouldn't do 99% of the time, and it's easy to do the right thing.

Of course, we still need the stream wrapper. But at least the absolute file URL problem is solved :)

jpamental’s picture

The main use case from my perspective was primarily image asset files, which would be nice to make relative to the theme itself. Favicons are one example but there could easily be others that are referenced via markup rather than CSS. It's also an issue when using the Web Font Loader to reference self-hosted CSS - that needs to be in a script written out inline in the page. I have to admit that I'm a bit unclear on where we are now.

Using base_path gives us a root-relative starting point, but if I'm understanding it correctly it seems like this works:

{{ file_url(directory ~ '/images/sample.jpg') }}

generates this:

http://domain.com/themes/mythemedirectory/images/sample.jpg

But this still leaves us with an absolute URL and not a relative one, so you will potentially get mixed http/https security messages.

Jeff Burnz’s picture

@Wim, thats great news, fantastic.

@jpamental thats right, but when #1494670 lands it will be relative, you can see this in the latest patch (109). Until then you can always work around this by setting something in preprocess, e.g. $variables['theme_path'] = base_path() . $variables['directory']; etc.

Wim Leers’s picture

Indeed. And note that #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send has already landed in 8.1.x, and in the next day or so it will be committed to 8.0.x.

jpamental’s picture

Great - thanks for the clarification @Jeff & @Wim

dawehner’s picture

Status: Needs review » Needs work

Mh this adds a function call to each executed template. You could save at least some time by putting it into \_template_preprocess_default_variables

Wim Leers’s picture

Title: base_path variable not available in html.html.twig » [PP-1] Document the recommended method of creating file URLs to theme assets (e.g. images)
Category: Feature request » Task
Priority: Normal » Major
Issue summary: View changes
Status: Needs work » Postponed

@dawehner see comments #18 through #33. The solution I proposed (and which seems to have agreement) is to use {{ file_url('theme://mytheme/img/llama.png') }}. Which means this is actually blocked on #1308152: Add stream wrappers to access extension files. Once that lands, all this needs to do, is improve documentation.

I updated the issue summary & title to reflect all this.

dawehner’s picture

Well, yeah, the question is whether these are the only usecases they need

Wim Leers’s picture

As this shows, the active_theme_path() Twig function (added in #2416857: Add an active_theme_path twig function) was a grave mistake.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.