[#17677] - [4.0] Change the lookup order for the favicon.ico
- Fixed in Code Base
- 19 May 2019
- Medium
- Build: 4.0-dev
- # 17677
- Diff
- Bakual:Change_Favicon_Order
- Success Hound No violations found. Woof! Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success continuous-integration/drone/pr the build was successful Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #17629 .
Summary of Changes
This PR changes the lookup order for the favicon so it first looks in the root folder and afterwards in the template one.
Currently, the lookup works the other way (first template, then root).
I think the current way has historic reasons because (I think) we used to ship a favicon.ico in root in earlier versions. The favicon in the template folder then allowed to override that favicon.
Today we don't ship a favicon in root, but we ship one in the template folders, thus it imho makes more sense to reverse the override place.
Testing Instructions
- Put different favicon.ico files into the root folder and the active template folder.
- Check which favicon is used
Expected result
The one from root is used, overriding the one from the template
Actual result
The one from the template is used, overriding the one from root.
Documentation Changes Required
Don't know if that hehavior is documented somewhere. If yes, it should be adjusted.
Also it should be documented as a changed behaviour between 3.x and 4.0.
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
The one from the template is used, overriding the one from root.
@Bakual I thought that templates are always the parts that make the final decision, e.g. js is overridden if there is file in template/js. So if we do this here then we have inconsistent behaviour, (unless I got this wrong)
EDIT: Ignore the above comment, this now aligns the behaviours
| Title |
|
||||||
Generally, you're right. But, the favicon is an awkward thing. If a template is shipping one, how do you override it at the template level? The entire problem we're having is users are building templates either from core or from a template provider who ships a favicon as part of the template, meaning it'll be overwritten on update unless it's excluded from updates. So how do you local override a template file?
| Labels |
Added:
?
|
||
Rebased PR, should be fine for testing again.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17677.
Thank you for considering the issue serious enough for a discussion.
I like to use of the user.ico file over the suggested root dir vs template dir location check. The reason for my preference is the following: On one standard site, two templates are being used -- one for the frontend (in protostar) and one for the administrator (backend, in isis). With the OP's original suggestion, two locations for two different favicon files are degraded to a single location (and single favicon). I'm using two favicons (both differing from the default Joomla favicon).
Note: The favicon for the backend is a visual negative of the frontend favicon.
Thank you for considering a different method and refraining from overwritting user favicons in the frontend template and backend template as well.
| Labels |
Added:
?
|
||
Rebased this PR
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17677.
@franz-wohlkoenig maybe you can test this PR again after the latest rebase to get it merged
@sanderpotjer i'm waiting for further on beta as there upgrades from nightly to nightly are possible.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17677.
| Status | Pending | ⇒ | Ready to Commit |
| Labels |
Added:
?
Removed: ? |
||
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17677.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17677.
| Labels | |||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-05-19 07:23:36 |
| Closed_By | ⇒ | roland-d | |
| Labels |
Added:
?
|
||
Thank you.
This needs documenting - not just as a change but as a feature - otherwise there is no point in the pr ;)