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

[bug] Favicon invalid path [fixes #979] #987

Merged
merged 1 commit into from
Oct 18, 2015

Conversation

mleanos
Copy link
Member

@mleanos mleanos commented Oct 15, 2015

Removed {{url}} from the Favicon path. This fixes the intermittent issues with the path resolving to an invalid location.

This seems to fix the bug with the Favicon invalid path. However, as discussed in #979, there are still remaining questions as to why these lines are using locals.url and locals.logo with the META tags
https://github.com/meanjs/mean/blob/master/modules/core/server/views/layout.server.view.html#L17-L29

Does anyone have any insight into these, and how we should address these issues? I think it would be appropriate to address all these issues in this PR. We just need to decide on how.

@ilanbiala ilanbiala added this to the 0.4.2 milestone Oct 15, 2015
@mleanos
Copy link
Member Author

mleanos commented Oct 16, 2015

@lirantal I removed the {{url}} from the twitter:image & og:image META tags.

Can someone shed some light on what local.url is intended for?
https://github.com/meanjs/mean/blob/master/config/lib/express.js#L44

Is this for redirection purposes? In order to know where the user came from?

@codydaig
Copy link
Member

LGTM

@lirantal
Copy link
Member

LGTM @mleanos
As to the locals question: http://expressjs.com/api.html#res.locals - it was added way back from Amos's 0.4 branch IIRC.

@lirantal lirantal self-assigned this Oct 17, 2015
Removed the {{url}} from the Favicon path. This fixes the intermittent
issues with the path resolving to an invalid location.

Removed the url from the twitter:image & og:image tags, to be static
references to the logo.
@mleanos
Copy link
Member Author

mleanos commented Oct 17, 2015

@lirantal Thank you for clarifying that. I've rebased & squashed.

If we need to address any of the other META tags, then I can do so. Otherwise, I think this is ready.

lirantal added a commit that referenced this pull request Oct 18, 2015
[bug] Favicon invalid path [fixes #979]
@lirantal lirantal merged commit 3a4c51a into meanjs:master Oct 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants