Skip to content
This repository was archived by the owner on Jan 7, 2025. It is now read-only.

Conversation

@Adnan-cds
Copy link
Contributor

The base_path Twig variable used in html.html.twig doesn't actually exist. This fix adds the localgov_front_page_default_lang variable as a replacement. As an added bonus, localgov_front_page_default_lang can be modified through PathProcessors plugins.

The *base_path* Twig variable used in html.html.twig doesn't actually exist.
This fix adds the *localgov_front_page_default_lang* variable as a replacement.
@stephen-cox
Copy link
Member

The base_path variable looks to be working fine on the demo site: view-source:https://demo.localgovdrupal.org/. Are these new variables going to be used for anything else?

Just to add, localgov_theme is about to become deprecated in favour of localgov_base

@Adnan-cds
Copy link
Contributor Author

Hi Steve,
Thanks for looking into this. Sorry, I should have added reproduction steps:

$ mkdir -p /tmp/html/
$ cd /tmp/html/
$ composer create-project --stability dev localgovdrupal/localgov-project
$ cd localgov-project/
$ > web/sites/default/settings.lando.php
$ ./bin/drush site:install --yes --db-url=sqlite://sites/default/files/.ht.sqlite localgov
$ php -S localhost:8888

view-source:http://localhost:8888/web/ should expose the problem with the non-existent favicon path. You can also check the presence of the favicon in the browser's title bar.

The reason base_path has no existence within html.html.twig is because neither _template_preprocess_default_variables() nor template_preprocess_html() declares it :(

Copy link
Member

@stephen-cox stephen-cox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the issue now. Happy to approve

@Adnan-cds
Copy link
Contributor Author

Thanks Steve. Merging...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants