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

Remove unnecessary Iterator loop in history.get #517

Merged
merged 1 commit into from May 19, 2016
Merged

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented May 18, 2016

This seems to be not necessary and it is causing issues particularly in
firefox 47, there query is a "XPCWrappedNative_NoHelper" and we can't
modify it which is what this loop tries to do.
Wrapping it in a try/catch does fix the issue as well but removing it
altogether seems to also fix it and seems that there are no adverse
effects.

Can someone give this a shot on various some versions of firefox?

This seems to be not necessary and it is causing issues particularly in
firefox 47, there query is a "XPCWrappedNative_NoHelper" and we can't
modify it which is what this loop tries to do.
Wrapping it in a try/catch does fix the issue as well but removing it
altogether seems to also fix it and seems that there are no adverse
effects.
@timss
Copy link
Member

timss commented May 18, 2016

👍 for FF 45.1.1 (:history, commands, hotkeys, ..).
If it works for you on FF 47 I wouldn't expect other current releases to be radically different.

I just reinstalled my desktop so I don't have my VMs to test different versions, unfortunately.

@gkatsev
Copy link
Member Author

gkatsev commented May 19, 2016

Going to merge this. We can always make another patch.

@gkatsev gkatsev merged commit afe333b into master May 19, 2016
@gkatsev gkatsev deleted the fix-history-get branch May 19, 2016 00:02
@timss
Copy link
Member

timss commented May 19, 2016

Sounds good!

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

Successfully merging this pull request may close these issues.

None yet

2 participants