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.

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

Conversation

wxiaoguang
Copy link
Contributor

Close #20240

At the moment, Firefox (iOS) (10x) has an engine bug. See #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.

This PR ignores such nonsense error event.

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Is FirefoxIOs aware of this bug?

I guess we should just handle this bug though - so LGTM.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 5, 2022
@wxiaoguang
Copy link
Contributor Author

I have submitted an issue https://github.com/mozilla-mobile/firefox-ios/issues/11191 , not sure when they can get it fixed.

@@ -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

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) ......

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 5, 2022
@zeripath zeripath merged commit f5c9717 into go-gitea:main Jul 5, 2022
@zeripath
Copy link
Contributor

zeripath commented Jul 5, 2022

Please send backport

@zeripath zeripath added this to the 1.18.0 milestone Jul 5, 2022
@wxiaoguang wxiaoguang deleted the bypass-firefox-ios-bug branch July 5, 2022 12:28
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jul 5, 2022
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Jul 5, 2022
* go-gitea#20240

At the moment, Firefox (iOS) (10x) has an engine bug. See go-gitea#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.

This PR ignores such nonsense error event.

Fix go-gitea#20240
@wxiaoguang wxiaoguang added the backport/done All backports for this PR have been created label Jul 5, 2022
zeripath pushed a commit that referenced this pull request Jul 5, 2022
Backport #20244 

* #20240

At the moment, Firefox (iOS) (10x) has an engine bug. See #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.

This PR ignores such nonsense error event.

Fix #20240
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 7, 2022
* upstream/main:
  Modify milestone search keywords to be case insensitive (go-gitea#20266)
  Fix toolip on mobile notification bell (go-gitea#20270)
  Allow RSA 2047 bit keys (go-gitea#20272)
  Refix notification bell placement (go-gitea#20251)
  Bump mermaid from 9.1.1 to 9.1.2 (go-gitea#20256)
  EscapeFilter the group dn membership (go-gitea#20200)
  Only show Followers that current user can access (go-gitea#20220)
  Init popup for new code comment (go-gitea#20234)
  Bypass Firefox (iOS) bug (go-gitea#20244)
  Adjust max-widths for the repository file table (go-gitea#20243)
  Display full name (go-gitea#20171)
  Adjust class for mobile has the problem of double small bells (go-gitea#20236)
  Adjust template for go-gitea#20069 smallbell (go-gitea#20108)
  Add integration tests for the Gitea migration form (go-gitea#20121)
  Allow dev i18n to be more concurrent (go-gitea#20159)
  Allow enable LDAP source and disable user sync via CLI (go-gitea#20206)
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 9, 2022
* go-gitea#20240

At the moment, Firefox (iOS) (10x) has an engine bug. See go-gitea#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.

This PR ignores such nonsense error event.

Fix go-gitea#20240
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
* go-gitea#20240

At the moment, Firefox (iOS) (10x) has an engine bug. See go-gitea#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.

This PR ignores such nonsense error event.

Fix go-gitea#20240
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Javascript error on mobile Firefox (iOS)
4 participants