Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Favicon option unnessecary? #27

Open
robwent opened this issue Sep 19, 2018 · 7 comments
Open

Favicon option unnessecary? #27

robwent opened this issue Sep 19, 2018 · 7 comments

Comments

@robwent
Copy link

robwent commented Sep 19, 2018

If the theme folder contains a file called favicon.ico then Joomla loads it automatically.

As the field has a default value, you can't stop the theme from loading it as it will default to /templates/bootstrap4/favicon.ico even if it doesn't exist.

@Dade88
Copy link

Dade88 commented Oct 19, 2018

Probably turns usefull when someone use tools like https://realfavicongenerator.net/ and want to have favicons elsewhere than site root or template root (not my case tbh).

@robwent
Copy link
Author

robwent commented Oct 19, 2018

Maybe just remove the default value then so it can be removed.

I guess that's not a big issue anyway since people who don't need it can just delete the option.

@jwiesel
Copy link
Contributor

jwiesel commented Oct 25, 2018

Hi @robwent ,
not sure if I got your point correctly.
The template offers to load a favicon from a different location than the template location itself. e.g. "/media/favicon.ico"
By that the template is update-proof - meaning: If you install a new version of the bootstrap4 template all files including the favicon will be replaced.
By copying your own favicon into the "templates/bootstrap4" folder you would lose the favicon file on every update.
Removing the default would mean, that your page does not have a favicon at all. Is this what you are aiming for?

@robwent
Copy link
Author

robwent commented Oct 25, 2018

The favicon option has a default value set here: https://github.com/sniggle/joomla-bootstrap4-template/blob/master/bootstrap4/templateDetails.xml#L67

When you set a default value, it's not possible for the user to save a blank field.

Therefore, the check to see if the field has a value set here: https://github.com/sniggle/joomla-bootstrap4-template/blob/master/bootstrap4/index.php#L61 will always return true.

Since the default points to the same place Joomla loads the favicon from anyway, you just get 2 favicon lines in the head of the doc.

The best option to me seems to be to remove the field and the favicon.ico from the theme. Then the user can add their own to the template (Which won't get replaced on an update).
Since it's a dev theme I doubt anyone is going to update it. If they did then they probably aren't going to want their favicon to be replaced by a default one.

If you think the option is a good idea, then the default value should be removed and the favicon.ico removed from the theme. Then Joomla won't load up the favicon that exists in the theme and the theme will only add the favicon line if the user adds a value to the field.

@Dade88
Copy link

Dade88 commented Oct 25, 2018

Just to add some info, it appears that most browsers check for favicons on the site root anyway.

@robwent
Copy link
Author

robwent commented Oct 25, 2018

Joomla also adds the favicon link if one exists in the root of the site.

If there is one in the root of the site, Joomla will add that.
If there is one in the theme, it will load that instead, overriding the one in the root.

@jwiesel
Copy link
Contributor

jwiesel commented Jan 3, 2020

Hi @robwent ,
I just reproduced the issue you mentioned, that Joomla! adds two favicons, if I load it from a different location. As this happens via a Joomla! library, a template override cannot prevent that.

I assume some users use the template without customization. As I don't want to remove that feature for them, I removed the default value to override the favicon only if intended.
By that for developers the duplicate entry is gone.
If people still want to set their own favicon the site HTML still contains two favicon entries, but the later one will be displayed by the web-browser: see here: https://theaterverein-mastershausen.de/

jwiesel added a commit that referenced this issue Jan 3, 2020
… entry in HTML, if not overridden. Added hint to template config page accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants