Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Variable i gets reset to 0 #4065

Closed
zznidar opened this issue Dec 26, 2018 · 6 comments
Closed

Variable i gets reset to 0 #4065

zznidar opened this issue Dec 26, 2018 · 6 comments
Assignees
Milestone

Comments

@zznidar
Copy link

zznidar commented Dec 26, 2018

JavaScript variable i is reset to 0 after incrementing it by 1 on an xmlhttp.onprogress (onerror) event. If it is given a different name, test for example, it is not reset to 0.

In Chrome 71.0.3578.99 for Android and Chrome, Firefox, Edge for Windows, variables are incremented regardless of the name.

Demo:
https://zznidar.github.io/bugreports/Firefox%20Focus/Variable%20i%20gets%20reset%20to%200/

Source code:
index.html: https://raw.githubusercontent.com/zznidar/bugreports/master/Firefox%20Focus/Variable%20i%20gets%20reset%20to%200/index.html
runtime.js: https://raw.githubusercontent.com/zznidar/bugreports/master/Firefox%20Focus/Variable%20i%20gets%20reset%20to%200/runtime.js

Steps to reproduce

  1. Visit https://zznidar.github.io/bugreports/Firefox%20Focus/Variable%20i%20gets%20reset%20to%200/
  2. Press the "Begin" button (Note: make sure JavaScript is enabled)
  3. Wait a few seconds for an alert to appear
  4. Press the "Begin" button again
  5. Wait for another alert. Compare variables i and test

Expected behavior

Each variable, regardless of its name, should be incremented when variable++ is executed in js-code. Thus, both i and test should be increased by 1 each time an alert pops up.

Actual behavior

Variable named i is reset to 0 each cycle, hence the alert always outputs it as 1. Variable test increments as expected.

Device information

  • Android device: pocophone f1, Android 8.1

  • Focus version: 8.0.4

  • Android device: Samsung Galaxy S8+, Android 8.0

  • Focus version: 7.0.13

@vesta0 vesta0 added the bug label Dec 28, 2018
@colintheshots
Copy link
Contributor

Which user agent is this happening with? Could you please send us the result of "focus:about"?

@zznidar
Copy link
Author

zznidar commented Dec 28, 2018

user agent:
Mozilla/5.0 (Linux; Android 8.1.0) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Focus/8.0.4 Chrome/71.0.3578.99 Mobile Safari/537.36

focus:about

8.0.4 (Build #323512256)

Firefox Focus puts you in control.

Use it as a private browser:

Search and browse right in the app
Block trackers (or update settings to allow trackers)
Erase to delete cookies as well as search and browsing history
Firefox Focus is produced by Mozilla. Our mission is to foster a healthy, open Internet.
Learn more

Second device (SGS8+):
user agent:
Mozilla/5.0 (Linux; Android 8.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Focus/7.0.13 Chrome/71.0.3578.99 Mobile Safari/537.36

focus:about
7.0.13 (Build #323021954)

Firefox Focus puts you in control.

Use it as a private browser:

Search and browse right in the app
Block trackers (or update settings to allow trackers)
Erase to delete cookies as well as search and browsing history
Firefox Focus is produced by Mozilla. Our mission is to foster a healthy, open Internet.
Learn more

@colintheshots
Copy link
Contributor

You're right. This is our fault and it's limited to WebView. It's caused by inserted JS polluting the namespace. We're moving to GeckoView soon, so we need to decide if we should fix this directly or simply change engines to fix it. Don't worry, we're on it.

@cpeterso
Copy link
Collaborator

cpeterso commented Jan 2, 2019

 "for (i = 0; i < links.length; i++) {" +

Can this bug be fixed by simply declaring let i in the loop head? The current code is implicitly using a global variable i. Breaking web content could cause some pretty bad problems.

This typo would cause an exception if this code used JS use strict mode.

@vesta0 vesta0 added size S 1 day or less of work P1 labels Jan 8, 2019
@vesta0 vesta0 added this to the v9.0 Release milestone Jan 8, 2019
@vesta0 vesta0 added P2 and removed P1 labels Jan 8, 2019
@colintheshots colintheshots self-assigned this Jan 8, 2019
colintheshots added a commit to colintheshots/focus-android that referenced this issue Jan 8, 2019
@ghost ghost added review and removed P2 labels Jan 8, 2019
@ghost ghost removed the review label Jan 8, 2019
@niftylettuce
Copy link

I had the same issue, I was using underscore.string globally in window and the global s variable was getting reset to 0 instead of being an object with methods like s.isBlank.

Using latest version of Firefox Klar

Ref: https://github.com/epeli/underscore.string

@niftylettuce
Copy link

To clarify - I am still having this issue on latest versions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants