-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Inline manifest.json #14038
Inline manifest.json #14038
Conversation
Is the data uri always going to work? |
Yes, browsers are required to implement data URIs if they want to conform HTML5. We just need to be careful with data emitted in the string because it must be URL-escaped. I guess |
Okay everything is escaped now including the URLs. Browsers will URL-decode the string before parsing it is it's safe to do in all cases. |
main.go
Outdated
@@ -41,7 +40,6 @@ var ( | |||
func init() { | |||
setting.AppVer = Version | |||
setting.AppBuiltWith = formatBuiltWith() | |||
setting.AppStartTime = time.Now().UTC() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems unrelated, could you describe why this is being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I's becoming unused because the /manifest.json
route was the only consumer of the variable (I added it with the HTTP cache commit). We can keep it around if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think it's duplicate with this line also.
Line 43 in 4aabbac
startTime = time.Now() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible keep the file but insert its contents directly into the attribute? That way you can keep the endpoint around for those who don't update their templates |
Can't insert the JSON as-is because it needs URL escaping and I'm generally leaning to not do that kind of extra processing for the sake of performance. What is the issue exactly about people not updating their templates? I thought templates work by the way of inserting user content at strategic places in the HTML. How come a user would have these parts hardcoded in their template customization? |
It could be put in |
I was thinking about generating it once on app startup and caching it in memory for even more speed, thought I guess the gains would be minimal. |
Please resolve the conflicts. |
Improve performance by eliminating this separate request and just inline this small JSON in HTML directly as a data uri. Also update previously static app name scripts to use AppName. I've confirmed this as working via "Add to Homescreen" feature which offered to save the shortcut under the new app name.
3f85e7d
to
47f4362
Compare
I am keeping around a custom template. |
Ah, so you are customizing the "private" templates, didn't know that was possible. You can just delete it, it doesn't exist anymore after this change. |
well that's too bad because I'm doing a little customization in there, such as using relative |
I think you could find other way to do that on Gitea. |
yeah, the expression will have to be replaced on the proxy. |
Improve performance by eliminating this separate request and just inline this small JSON in HTML directly as a data uri. Also, I've updated the previously static app name scripts to use
AppName
from config.theme_color
is removed because it's duplicating with the existing meta tag.I've confirmed this as working via "Add to Homescreen" feature which offered to save the shortcut under the new app name.