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

Bypass Firefox (iOS) bug #20244

Merged
merged 2 commits into from
Jul 5, 2022
Merged

Bypass Firefox (iOS) bug #20244

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee0bfa4
Bypass Firefox (iOS) bug
wxiaoguang Jul 5, 2022
0b471d9
Merge branch 'main' into bypass-firefox-ios-bug
wxiaoguang Jul 5, 2022
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
5 changes: 5 additions & 0 deletions web_src/js/bootstrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export function showGlobalErrorMessage(msg) {
* @param {ErrorEvent} e
*/
function processWindowErrorEvent(e) {
if (!e.error && e.lineno === 0 && e.colno === 0 && e.filename === '' && window.navigator.userAgent.includes('FxiOS/')) {
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 the last condition unnecessary?
It feels less like a workaround without the last condition...

Suggested change
if (!e.error && e.lineno === 0 && e.colno === 0 && e.filename === '' && window.navigator.userAgent.includes('FxiOS/')) {
if (!e.error && e.lineno === 0 && e.colno === 0 && e.filename === '') {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But only Firefox(iOS) has this bug. 'FxiOS/' means Firefox(iOS)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent/Firefox#firefox_for_ios

Firefox for iOS

Firefox for iOS uses the default Mobile Safari UA string, with an additional FxiOS/ token on iPod and iPhone

Copy link
Member

Choose a reason for hiding this comment

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

At the moment.
But if Firefox already manages to produce such an obscure bug, why shouldn't the others be able to do that as well?

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with both approaches, hence I already approved, but I still wanted to point that out.

Copy link
Contributor Author

@wxiaoguang wxiaoguang Jul 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I do not think it could be a general bug for modern browsers. This bug is highly likely to be caused by some incorrect DOM layout handling. The last similar bug in my mind is as old as back to Internet Explorer 6 (20 years ago) ......

// At the moment, Firefox (iOS) (10x) has an engine bug. See https://github.com/go-gitea/gitea/issues/20240
// If a script inserts a newly created (and content changed) element into DOM, there will be a nonsense error event reporting: Script error: line 0, col 0.
return; // ignore such nonsense error event
}
showGlobalErrorMessage(`JavaScript error: ${e.message} (${e.filename} @ ${e.lineno}:${e.colno}). Open browser console to see more details.`);
}

Expand Down