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.

Sign up for GitHub

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

Jump to bottom

Simplify the error message when index.js couldn't be loaded #22354

Merged
merged 8 commits into from
Jan 18, 2023
Merged

Simplify the error message when index.js couldn't be loaded #22354

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9c4984f
Update footer.tmpl
wxiaoguang Jan 6, 2023
4b63ff1
Update templates/base/footer.tmpl
wxiaoguang Jan 7, 2023
d704c1c
Merge branch 'main' into wxiaoguang-patch-1
wxiaoguang Jan 7, 2023
e71566e
Merge branch 'main' into wxiaoguang-patch-1
zeripath Jan 16, 2023
921bc34
Merge branch 'main' into wxiaoguang-patch-1
lunny Jan 17, 2023
5d8a29e
Merge branch 'main' into wxiaoguang-patch-1
lunny Jan 17, 2023
e5d48e8
Merge branch 'main' into wxiaoguang-patch-1
jolheiser Jan 17, 2023
f6e6f7e
Merge branch 'main' into wxiaoguang-patch-1
lunny Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion templates/base/footer.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<script src='https://hcaptcha.com/1/api.js' async></script>
{{end}}
{{end}}
<script src="{{AssetUrlPrefix}}/js/index.js?v={{AssetVersion}}" onerror="alert('Failed to load asset files from ' + this.src + ', please make sure the asset files can be accessed and the ROOT_URL setting in app.ini is correct.')"></script>
Copy link
Member

@delvh delvh Jan 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But wasn't this the exact reason why you added this alert, especially the ROOT_URL, in the first part?
Given that it is still often enough the reason, I think we should keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not. See the original PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't {{AssetUrlPrefix}} built from ROOT_URL ? Or some other app.ini setting ? It would be useful to be hinted about which app.ini setting affects that url...

Copy link
Contributor Author

@wxiaoguang wxiaoguang Jan 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't {{AssetUrlPrefix}} built from ROOT_URL ?

No, not directly.

Or some other app.ini setting ?

Somewhat, but not directly. See the PR's comment: ex: https://gitea.com/gitea/helm-chart/issues/392

It would be useful to be hinted about which app.ini setting affects that url...

There are many different cases. In most cases, users could fix it by themselves. And in most cases, the problem is caused by user's environment (the example in the PR's comment). If not, it's better to collect common problems and write a FAQ.

<script src="{{AssetUrlPrefix}}/js/index.js?v={{AssetVersion}}" onerror="alert('Failed to load asset files from ' + this.src + '. Please make sure the asset files can be accessed.')"></script>
{{template "custom/footer" .}}
</body>
</html>