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

FastBack can cause a script to run twice #1083

Closed
arantius opened this Issue Mar 1, 2010 · 7 comments

Comments

Projects
None yet
2 participants
@arantius
Copy link
Collaborator

arantius commented Mar 1, 2010

See: http://groups.google.com/group/greasemonkey-users/t/5f59e19b63aa4b1b
And: https://bugzilla.mozilla.org/show_bug.cgi?id=548145

In essence, the FastBack feature combined with the way Greasemonkey tests the current URL (and whether the script should run) can in certain instances cause the script to run twice on the same page.

I personally consider this a bit of an edge case, but whether it's a Moz bug or not, if we can fix it as easily as comparing a different URL, we should.

@qufighter

This comment has been minimized.

Copy link
Contributor

qufighter commented Jul 22, 2011

I'm fairly confident we can detect this sort of FastBack in the TabsProgressListener onLocationChange added for document start although that would probably not be very pretty unless we attached some property to a safe window object that the contentLoad listener would check (assuming safe win is actually safe). Ideally the DOMContentLoaded event has some property that we could check.

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jul 23, 2011

This was solved by adffbed right?

@qufighter

This comment has been minimized.

Copy link
Contributor

qufighter commented Jul 23, 2011

You could be right. On bugzilla above they deduced that the event was firing for the page that was being left, yet somehow the event had different URLs, one for the fastback page and one for the foreward page.

e.target.documentURI: http://www.last.fm/     //firing for this page that we just left
e.target.defaultView.location.href: http://vestitools.pbworks.com/f/gmbugtest.html    //fastback to this page where we look at this URL and possibly take redundant action

We do not use documentURI as they suggested but I notice some href changes in the commit, however not with regard to GM_BrowserUI.contentLoad or GM_BrowserUI.gmSvc.runScripts functions.

we use the following method (gmSvc.runScripts, greasemonkey.js):

var url = aWrappedContentWin.document.location.href;
if (!GM_getEnabled() || !GM_isGreasemonkeyable(url)) return;

Then we run matching scripts for that URL. I would almost suggest:

if (href == event.target.documentURI)
  GM_BrowserUI.gmSvc.runScripts('document-end', safeWin, window);

browser.js ~ line 108

@qufighter

This comment has been minimized.

Copy link
Contributor

qufighter commented Jul 23, 2011

The above reference should be deleted, that belongs to a branch that no longer exists.

I fixed my git, ended up creating a new_master locally from an old commit and then updating it, this comparison should work correctly now:

https://github.com/qufighter/greasemonkey/compare/issue-1083

qufighter added a commit to qufighter/greasemonkey that referenced this issue Jul 23, 2011

this branch relates to greasemonkey#1083 and will prevent scripts fro…
…m running during simultaneous load and fastback conditions

qufighter added a commit to qufighter/greasemonkey that referenced this issue Jul 24, 2011

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Jul 25, 2011

Can you provide some steps to reproduce this bug? (And thereby verify the fix...)

@qufighter

This comment has been minimized.

Copy link
Contributor

qufighter commented Jul 25, 2011

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12

Using the test posted here: http://vestitools.pbworks.com/f/gmbugtest.html
and this test script: http://vestitools.pbworks.com/f/gmbugtest.user.js

Click on "Link to another site"
As soon as the favicon and URL for the new site show up in the URL bar, press back,
the DOMContentLoaded will fire for lastfm.com (especially if the page has been previously loaded)
The script on the first page gmbugtest.html will run twice.

I confirmed that the patch a04ae31 prevented the script from executing more than once.

I have not yet verified this issue exists in firefox 5 but may check it out, I'm not incredibly thrilled about adding more conditional logic but I think it makes sense to ensure we are on the page that the event fired for before running scripts.

I remember encountering this issue on my own a while back but couldn't reproduce it. It would be nice to have it go away since I often press a link and change my mind about that link in the time it takes to load.

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Aug 5, 2011

Bug and fix confirmed in FF 5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment