Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Favicon incorrect path #979

Closed
mleanos opened this issue Oct 13, 2015 · 5 comments
Closed

Favicon incorrect path #979

mleanos opened this issue Oct 13, 2015 · 5 comments
Assignees
Milestone

Comments

@mleanos
Copy link
Member

mleanos commented Oct 13, 2015

I've experienced an issue with the favicon resolving to an incorrect path. It happens intermittently, and I'm not able to reproduce regularly. However, I have seen others reporting the same issue.

When the issue occurs, the favicon URL resolves to http://localhost:3000/articlesmodules/core/client/img/brand/favicon.ico or similar; this will depend on what route I navigate to. So in this example, I was on the Articles route. I've seen it when I was on the Profile route as well.

The problem seems to be with this line here..
https://github.com/meanjs/mean/blob/master/modules/core/server/views/layout.server.view.html#L32

If I remove the {{url}} from the href, I am unable to reproduce the issue; and not for a lack of effort. Can someone shed some light on why the {{url}} was added to this path?

Also, I'm confused as to what these META tags purposes are.
https://github.com/meanjs/mean/blob/master/modules/core/server/views/layout.server.view.html#L17-L29

It seems that the {{url}} was definitely put there for a specific purpose. I can only guess that it's to accommodate an environmental change.

I think the favicon path shouldn't need the {{url}}, but I'm not sure of the other above mentioned uses.

I'd like to fix this, but before I submit a possible fix for the favicon, I'd like to get a bit more context & insight from others.

@vaucouleur
Copy link
Contributor

Same here. I can confirm that I also had intermittent issues with favicon. @mleanos made a very good description of the issue, I could not isolate the issue further.

As mentioned by @mleanos, if you look closely at the GET requests, you will see the favicon being requested at some very wrong locations:

GET /authentication/signinmodules/core/client/img/brand/favicon.ico 200 14.889 ms - -
... then.. later on.. browsing around..
GET /admin/usersmodules/core/client/img/brand/favicon.ico 200 8.246 ms - -

Note, also, the missing "/" above.

@ilanbiala
Copy link
Member

@mleanos @vaucouleur Quick check of git blame shows that ef3a3f9 modified the file when we fixed the formatting, but the {{url}} was also there. I think it's a typo that got inserted by mistake. Regarding the META tags, do they not do anything for Facebook/Twitter anymore?

@vaucouleur
Copy link
Contributor

@ilanbiala Yes, probably a typo. The previous commit inserted the {{url}} and the favicon variable:

d7f9622#diff-76401fabda693ae2a74744450d655053

Not sure about the other META tags.

@mleanos
Copy link
Member Author

mleanos commented Oct 14, 2015

@ilanbiala Regarding the META tags, I don't really know what their purpose is & the current implementation adds to the confusion. Why is the brand logo being used for the Twitter image?

I suppose I can submit the fix for the favicon, and we can address the META tags as well; however, I'll need more feedback & context with the latter.

@lirantal
Copy link
Member

That PR should also fix the logo reference to {{logo}} (see https://github.com/mleanos/mean/blob/favicon-path-bug/modules/core/server/views/layout.server.view.html#L22) because both favicon and logo are basically static assets, not related to the current URL.

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

No branches or pull requests

4 participants