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

@run_at document_start #1103

Closed
qufighter opened this issue Apr 12, 2010 · 63 comments
Closed

@run_at document_start #1103

qufighter opened this issue Apr 12, 2010 · 63 comments
Milestone

Comments

@qufighter
Copy link
Contributor

//greasemonkey.js
domContentLoaded: function(wrappedContentWin, chromeWin, injectionTypeFlag) {
...
}

It is my understanding that gmIGreasemonkeyService.xpt is not allowing easy extension of the function shown above, making it impossible to call with additional parameters from browser.js. It will be necessary to tell which scripts to init at document-start and which scripts to init at content-loaded since this function will potentially be called more than once by different even listeners. I propose a 3 flag system where

0: Inject all scripts (in case of early injection disabled)
1: Inject only non document-start scripts (in case some injected already at doc-start)
2: Inject only document-start scripts

Boolean would probably work just as well especially if null|false|true, however if we were to support more run-at states in the future then it would be more readily extend-able as an int or a byte.

I would probably just pass the flag into an extended initScripts function that seeks only the right type scripts, but I don't expect any difficulty in accomplishing that. I'm not sure how to extend the gmIGreasemonkeyService.xpt to allow more parameters for domContentLoaded or allow access to an alternate function that would support an additional parameter or object.

An alternative would be allow the wrappedContentWin object to be extended with additional attributes. I'm excited to post this fork but i also want it to work first without cheating and adding attributes to the unsafeWindow object. While we might not get this in before the real-time metadata parsing I am hoping we can support this additional parameter before then or point me in the right direction.

@arantius
Copy link
Collaborator

The function you describe, by its name, is the one that is called from the "DOMContentLoaded" event handler. If you're intending to run something which is not fired from that event, you should not be calling this function. If you want to do essentially the same thing (inject scripts) from two different places/in two different ways, there probably needs to be some refactoring, so the common stuff exists once, simply called twice.

There should be no "inject these types of scripts" parameters, but each case is handled by calling the appropriate function, which by its nature knows what scripts are concerned.

Even if there is a flag, it should be a series of named constants, not just "0", "1" and so on.

That said, yes. Components must adhere to an interface. There's a trick to get around that, and I think we're even using it, but it has caveats. So it's still best to document it properly, through the standard interface definitions.
https://developer.mozilla.org/en/XPIDL

See the "idl" files in the source directory of that name.

An alternative would be allow the wrappedContentWin object to be extended

Again, absolutely not. This object has a very distinct meaning from its name, and it should NOT be used just to "pass parameters" not related to its named function. (And anyway, being a XPCNW, it also must adhere to its interface, you can't just tack anything you like onto it.)

@qufighter
Copy link
Contributor Author

Thank You! I've found the correct file (in my haste/lack of bash I was modifying the public xpi), I will make a separate function later we could remove the existing one. If only there were an easy to use idl->xpt compiler on my system it would be ready, I still need to satisfy some prerequisites or build one. The format of the xpt file doesn't look that difficult so I should be able to get my testing working either way. I will probably call the function handleInjectionState or something similar injectScriptsForStage and it will accept three parameters. As far as naming goes I'm open to suggestion.

I agree about named constants however defining constants that are exposed to both greasemonkey.js and browser.js in a single place seems like another challenge.

INJECT_STATE_ALL, INJECT_STATE_DOMLOAD, INJECT_STATE_DOCSTART

If we should plan on supporting additional states such as document-end or document-idle is another question although the most useful one at the moment is document-start there are a lot more possibilities.

@arantius
Copy link
Collaborator

If only there were an easy to use idl->xpt compiler on my system it would be ready,

One of the less fun bits of XPI development. Googling "xpidl.exe" or "xpidl binary" can be helpful, if you trust what you find. I've saved a copy for my own use: http://arantius.info/downloads/xpidl.exe

I will probably call the function handleInjectionState or something similar

The existing function, domContentLoaded is called at domContentLoaded time. The new function, to run at document start time, might be called documentStart (the logic is tricky, eh?). They should both gather the list of appropriate scripts for the event they handle and, in turn, call helper functions to do most of what is today done in domContentLoaded.

I agree about named constants however defining constants that are exposed to both greasemonkey.js and browser.js in a single place seems like another challenge.

Add an object to the GM service object. Fill it with a few read only keys.

If we should plan on supporting additional states such as document-end or document-idle is another question

No. Our general policy is that we don't implement for the authors anything that they can implement themselves. That's the realm for @require -- which can be maintained much more nimbly, among other benefits.

@qufighter
Copy link
Contributor Author

This particular issue regarding compiling gmIGreasemonkeyService.xpt from gmIGreasemonkeyService.idl should be resolved now. Thanks for your help!
reverted commit
I have yet to merge in the rest of the implementation. I was planning on merging the config.js portion next. There are also additions in browser.js plus greasemonkey.js.

Is one huge commit preferred? I was thinking it would be easier to split into pieces as to not collide with other work that's being done, since there will possibly be a lot more debate about naming conventions the particular sections (especially config), but it's also easy enough to include these changes grouped in another commit instead.

I verified the 3 line breaks in the complied file posted correctly, also compiled and ran this in the latest version.

@arantius
Copy link
Collaborator

http://greasemonkey.github.com/contrib.html

What's best is one branch per contribution. With any number of commits. I, personally, find committing early and often helps move things forward. Small commits, with descriptive log messages on each, are easy to understand.

Given a branch, we can discuss it, changes can be added in the branch, and then the whole thing can be merged upstream. Start with a "master" branch that directly mirrors upstream's master (with no changes). Start a branch containing changes, isolated from said master.

@qufighter
Copy link
Contributor Author

Sounds great! I was wondering why everyones graph looked so nice. I've started posting some of the changes to a branch called document-start, and i got going to the point where it should be basically all done now. I posted a userscript to test with http://gist.github.com/418707 however I imagine there will possibly be some refactoring since we could eliminate the somewhat redundant initEarlyScripts versus initScripts function (could possibly be handled with a parameter if preferred) plus I had to change initScripts that existed before and split out an operation into another function, which is something that could also be done a couple places in browser.js as far as populating the menu with scripts coded block appearing twice.

I suppose I'll probably sleep on it for now. I didn't include the value I had originally planned for about:config to dissableEarlyInjection however I'm not sure it's necessary. There are a couple things we could still probably optimize but it works fine in my testing so far.

Here is a link to the commit history: commits

Here is the complete diff to date: document-start

I will periodically upload a diff here: diff file upload colorize

@sizzlemctwizzle
Copy link
Contributor

First off, I wanna say I'd really love this feature(as I'm sure many others would) and I applaud your efforts qufighter. Now getting down to business.

Having a global preference for injecting all script early sounds very evil to me. Most installed scripts would likely be broken since they rely on the DOMContentLoaded to ensure the document is ready to mess with.

Therefore in my branch I've made the enableDocumentStart pref basically allow scripts to specify their early injection via the metadata value. If this pref is false then no script will be injected early. Because this early injection feature is only available in Firefox 3.5 or greater, the enableDocumentStart pref will be set to false. There still needs to be some code to turn this feature on when a user migrate from a version of Firefox where this wasn't possible to a version where it is.

In addition I've made an effort to reduce duplicate logic in greasemonkey.js.

@sizzlemctwizzle
Copy link
Contributor

I will be keeping the diff here updated: http://vidzbigger.com/diff/colorize/index.php?file=document-start.txt

Or you could let GitHub do it for you: http://github.com/qufighter/greasemonkey/compare/master...document-start

Select the "Files Changed" tab.

@qufighter
Copy link
Contributor Author

I'm about to run out to dinner so this is going to be kind of quick I'll have to look more closely later. To clarify a couple things however:

scripts only run at document start if the
@run-at document-start
metablock imperative is present, this should not effect any existing scripts UNLESS those scripts have the @run-at defined

Secondly there is code that falls back, I've tested this in 2.0, and the preference value is defined as false for anything less than 3.5, so this feature is completely disabled on first run for anyone who has less than ff 3.5.

For anyone with 3.5 or greater the @run-at should take care of this being disabled by default. For ff 3.5 or greater about:config isn't even troubled with the value to enableDocumentStart unless the user explicitly adds it.

The only trouble I was thinking was that what if the try catch that attaches the documentstart listener fails for some other reason, then the ff3.5 user will be stuck with a disabled document start and not necessarily realize it, but that shouldn't ever happen anyway.

I agree if you would like to keep the value either defined for everyone or have it configured to false by default especially.

Thanks for the link to the actual compare feature! I definitely discovered it just a little too late :D

@sizzlemctwizzle
Copy link
Contributor

scripts only run at document start if the @run-at document-start metablock imperative is present, this should not effect any existing scripts UNLESS those scripts have the @run-at defined

I've looked over your code again and I believe you are correct. In which case, the changes in my branch are just re-factoring of the code.

@qufighter
Copy link
Contributor Author

I took a quick look at the changes I think it looks pretty solid and simpler. I'm sure it's just about ready. The whole file looks a lot better now.

@sizzlemctwizzle
Copy link
Contributor

One case that still needs to be addressed is migration. If a user starts using this with a version of Firefox less than 3.5, the feature will be disabled and enableDocumentStart will be set to false. Then later they upgrade and enableDocumentStart will still be set to false even though this feature is now possible.

Update: I've added code to turn this feature back on if a user migrates to Firefox 3.5 or greater.

@arantius
Copy link
Collaborator

arantius commented Jun 1, 2010

I've yet to read in detail, but based on the comments:

A) Don't worry about Firefox below 3.0, we're adjusting to that minimum version.
B) There shouldn't be any sort of "preference" for this. Either we support it or we do not. Any script meant to run at document start time will have to run then, and vice versa. Any situation that makes it work only some times (users, settings, firefox versions, etc) is not good enough.

@sizzlemctwizzle
Copy link
Contributor

A) Don't worry about Firefox below 3.0, we're adjusting to that minimum version.
Yes but according to qufighter this feature isn't available until Firefox 3.5.
B) There shouldn't be any sort of "preference" for this. Either we support it or we do not.
The preference is for Firefox below 3.5 where this will fail. And rather than keep failing we've set a preference to not use this feature in that case.

@arantius
Copy link
Collaborator

arantius commented Jun 1, 2010

Yes but according to qufighter this feature isn't available until Firefox 3.5.

What feature is "this" feature?

The preference is for Firefox below 3.5 where this will fail. And rather than keep failing we've set a preference to not use this feature in that case.

That doesn't invalidate my point. If it doesn't work 100% of the time for 100% of the users, I don't want it in GM. Can you imagine the documentation otherwise, not to mention the users (both end users and script authors) trying to deal with it?

@sizzlemctwizzle
Copy link
Contributor

What feature is "this" feature?
Running a script before the document has loaded.
That doesn't invalidate my point. If it doesn't work 100% of the time for 100% of the users, I don't want it in GM.
Okay. Well I might have found an older way to accomplish this so there may still be some hope.

@qufighter
Copy link
Contributor Author

The feature in specific is called addTabsProgressListener it is well documented that it is only supported in firefox 3.5 or newer.... addTabsProgressListener

I'm sure we can still keep it simpler than the documentation for the @run-at parameter here especially since we are only supporting one of the states (and probably the only one necessary)

PS the original specification the @run-at notion is based on is mentioned design-documents/user-scripts

@sizzlemctwizzle
Copy link
Contributor

I don't have the time to read all of that, but I did find an old method of using addProgressListener on each tab to achieve the same result as addTabsProgressListener. I've implemented it as a fallback in this branch. It is confirmed to work in Firefox 3.0 and I've also removed the now useless preference.

@qufighter
Copy link
Contributor Author

Well it looks great to me, ditching support for 1.5, 2.0 is going to make it a lot easier, I still think that simply running the script at the normal time if doc-start isn't available is also sufficient as far as this feature goes considering if you attach a DOMContentLoaded listener of your own in a doc-start script (basically a must) versus a regular script it still fires at the correct/same time. Doc start seems just for above and beyond optional type functionality of doing things more seamlessly, it's great if we can support the entire user-base with this feature!

@sizzlemctwizzle
Copy link
Contributor

I still think that simply running the script at the normal time if doc-start isn't available is also sufficient
No, arantius is right. We can't add half-assed supported features. However, a work around for calling scripts at doc-start is available for all versions of Firefox we support in this branch. Test script.

@arantius
Copy link
Collaborator

arantius commented Jun 1, 2010

I still think that simply running the script at the normal time if doc-start isn't available is also sufficient

If "simply running the script at the normal time" is sufficient, then we can just close this issue as a wontfix -- that's what GM does today every time. This feature does have significant value, but only if it works reliably. For:

  • Injecting content-scope functions/data that inline page scripts rely on, but don't work / are broken (e.g. which rely on Internet Explorer only features).
  • Injecting CSS for display, which will not "flicker" into view after the page (DOM) has loaded.

And so on. As for making this work reliably, I'd expect this can be done with way way less code. Look at some older versions of Firebug. Firebug injects a content-scope object (console) at document start time, via XBL (IIRC). Essentially, bind to the that holds the content page, use XBL to hook into the constructor event for that element, and you naturally have a reference to the window at exactly the time that the content window is being created.

Firebug is BSD licensed, which is MIT compatible, so we can reuse their code as long as we include the proper attribution and licensing nodes as per the BSD license's terms. If you investigate this route, I'd highly suggest looking at as old a version of FIrebug as you can -- their code got a lot more complex very quickly, and we don't need most of those complex features. I remember doing something like this this back in the 2007 or so time frame, so that's something like Firebug 0.4, maybe 1.0.

@sizzlemctwizzle
Copy link
Contributor

According to their wiki, they only started adding the console at document start in version 1.3, but they are pretty vague about what they used before 1.2 so perhaps they used the same method as you described. Anyway, I still have a lot more digging to do.

@qufighter
Copy link
Contributor Author

I've been looking around in firebug 1.5.3 since the start of this project but it is kind of complicated. Some thing to look at might be injector.attachIfNeeded, I'll have to locate the older source code soon

However in our own pre-existing listener that captures the user clicking tabs event also fires when the user changes pages (at document-start as close as I can tell) including page refresh (confirmed doc-start timing is correct in 3.5)

LOC2 [xpconnect wrapped (nsISupports, nsIDocShell, nsIWebNavigation, nsIDocShellHistory, nsIInterfaceRequestor, nsIWebProgress, nsIDocumentLoader, nsIRequestObserver)] || [xpconnect wrapped (nsISupports, nsIHttpChannel, nsIRequest, nsIChannel, nsIUploadChannel, nsITraceableChannel)] || [xpconnect wrapped nsIURI]

GM_BrowserUI.onLocationChange = function(a,b,c) {
  console.log('LOC2 '+a+ ' || '+b+ ' || ' +c,true)
  if (b) GM_BrowserUI.tabLocationChange(a.DOMWindow)

CONCLUSION: b is null during tab switching and has a value at ~document-start (fires before current implementation only slightly). I'm not sure how well or reliably...

Modified: b is null during tab switching EXCEPT during session restore or loading of inactive tabs

Same functionality seems to work, it has its own limitations though:

(added) Namely that it does not always fire for each tab on browser startup/restore, which our current implementation accounts for.

UPDATE: unfortunately B does not have a value during session restore (in inactive tabs), so it won't be enough on it's own I don't think, since during restore this needs to fire whether B is null or not, then immediately after that, fire only if B has a value.

UPDATE2: I managed to get our existing GM_BrowserUI.onStateChange listener for this firing.... turns out it fires too early for document start (the old document still exists) and then fires again (too late) after everything is loaded. (edited)

I'm going to re-state update1 that our pre-existing NOTIFY_DOCUMENT for the menu commander function GM_BrowserUI.onLocationChange DOES fire at the correct time, however, it only fires as expected in the active tab, so it does not work well during session restore (at least not yet, or not based on testing for B being null).

I undid all of the experiments and am going to test the current implementation FF3.0 next comment covers what I found there

FINAL UPDATE: I seem to have found this condition.... a.loadGroup.activeCount
if (b||(!b&&a.loadGroup&&a.loadGroup.activeCount>0))

a.loadGroup is either undefined or it's a.loadGroup.activeCount is zero when switching tabs, however the active count is 1 (or greater I assume). Any time b is defined it is safe to inject scripts, in the absence of b an a.loadGroup.activeCount>0 is sufficient to indicate scripts should occur. It is probably sufficient to:
if (a.loadGroup&&a.loadGroup.activeCount>0)
GM_BrowserUI.tabLocationChange(a.DOMWindow)

the PROBLEM still exists however, that sometimes tabs are grouped into the same loadGroup, in which case this doesn't seem to fire for all tabs during restore :/

SO I think I'm just going to test for the body tag, if there is no body tag, we can almost certainly assume a load is occurring and it's at document-start ....

the problem that this listener is still not firing for all tabs on startup still exists, although it does sometimes, and was firing fine what I was restoring 6 tabs (with progress listeners attached but not registered with the function where I was detecting them) but then only firing for one of them when I had 5 tabs (and set up to only use the "main" listener), which makes very little sense, but it seems like we DO need to attach listeners to each tab or at least listen for new tabs being added and attach the listeners ourselves then.

Unless we can latch onto the constructor that creates documents somehow, since it would have to petty much create a new instance of document no matter what, at least I'm pretty certain. In firefox source code there is a nifty line that throws the DOMcontentLoaded, however there is no corresponding line near the constructor (or a few functions in) to throw the DOMdocumentStart or all these would be more easily solved.

@qufighter
Copy link
Contributor Author

There is an odd bug with firefox3.0 portable where a certain tab (usually the first one that is opened when the browser is launched) does not fire the TabOpen event required for ff3.0 and the document-start listener never gets attached. During restore all tabs fire tab-open except the first one, so one tab fails to apply document start no matter how many times it's refreshed.

Notification_when_a_tab_is_added_or_removed but not when the tab is always there.

Somehow at startup we need to iterate existing tabs, I expect we will find one tab, and need to attach the listener to that. I am guessing the browser xul contains one tab by default so it never fires the tab creation event.

So it might just be a matter of doing this once

EDIT: this is fixed as far as handling the one tab that exists during startup (3.0) by this: commit since all the other tabs in the restore are added they fire the normal tabOpen event.

@qufighter
Copy link
Contributor Author

As I started to document this feature on the talk page and noticed there is a discrepancy between the design documents

http://wiki.greasespot.net/Talk:Metadata_Block

It appears @run-at document-start becomes @run_at document_start in the more recent document.

I have posted a commit containing this change unless we should support both. reverted

@arantius
Copy link
Collaborator

I would definitely like to make this mirror the same syntax Chrome uses, so scripts will work on both platforms:

http://www.chromium.org/developers/design-documents/user-scripts

// @run-at document-start

The document you linked from that talk page which uses underscores is about chrome extensions, not user scripts. They're very similar, but different.

@sizzlemctwizzle
Copy link
Contributor

Firebug injects a content-scope object (console) at document start time, via XBL (IIRC).
So I finally got the time to look into this more and I haven't found any evidence of this. All versions that I've looked at use a similar method of using progress listeners to tell when a new url has been navigated to that I use in my branch.

As I explained earlier the wiki details when they have modified the implementation of the console so I examined versions within those time frames of release.

Version 0.4 uses progress listeners and that code is located in firebug/content/firebug.js. Shortly after that they moved the progress listener code to it's own file: firebug/content/tabWatcher.js. I saw this in version 1.01, 1.3.0, and even the latest version 1.5.4.

@qufighter
Copy link
Contributor Author

I think we have a solid implementation, and attaching the listener is really only one line of code in firefox 3.5, it is added in a try block and the catch block is kind of large but handles firefox 3.0 legacy support and functions identically to the firefox 3.5 code.

The changes to date have been simplified and can be reviewed here

The only thing that was left is what happens during restore, since we are listening for tabOpen and adding our listener to the tab then, tabs that are already open before greasemonkey runs did not necessarily receive listeners in firefox 3.0 (since one tab always exists), I handled that issue in this commit However I think that's all it takes to solve it, since it's only the initial tab that doesn't seem to fire the tabOpen event during restore.

There may be an alternate implementation but it requires a lot of looping through tabs and only works for active tabs (not those that change location in the background) and I'm not really confident it's the best approach since it would be a lot easier to run things twice accidentally during restore since the script only works in the active tab under that approach.

@sizzlemctwizzle
Copy link
Contributor

I handled that issue in this commit
I don't know, this seems kind of a half-ass approach. I wrote this commit that adds progress listeners to all tabs already open.

@qufighter
Copy link
Contributor Author

Sounds great, I think that it's just in the Firefox xul that has one tab defined by default, I think they removed this in later releases, but your method is definitely more robust since there could be some variety of modification between different releases and no guarantee about when browser.js will execute that I'm aware of.

@arantius
Copy link
Collaborator

The loadflags seem to always be 7933952 regardless of real load or back/forward. Didn't bother to decipher what that means, because it's constant anyway. I'm seeing:

The aProgress parameter has a "loadType" which seems to be 1 or 4, or sometimes 2097153.

It appears to be from nsIDocShellLoadInfo:
http://www.oxymoronical.com/experiments/apidocs/interface/nsIDocShellLoadInfo
except that doesn't specify things to match the >2million case, which is 1<<21 + 1 -- that seems to be coming from:
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsIChannel.idl#211
where 1<<21 is LOAD_CALL_CONTENT_SNIFFERS (??).

Anyway, it seems that "& 4" seems to be a good "ignore this event" case.

@qufighter
Copy link
Contributor Author

The only other thing I can think to check is session restore, on foreground and background tabs.

@sizzlemctwizzle
Copy link
Contributor

Nice job managing to merge a branch that hadn't been touched in over a year. I'll have to take it for a spin when I get home.

Now that we're bumping the min version to 3.5 perhaps we can replace that XML file for storing script data with an SQLite database using the Storage api.

@Martii
Copy link
Contributor

Martii commented Jul 19, 2011

... replace that XML file for storing script data with an SQLite database

Still some issues with DB integrity upstream... Moz runs a periodic sweep to ensure that DB is intact so GM would probably need to do the same.

arantius added a commit to arantius/greasemonkey that referenced this issue Jul 19, 2011
@arantius
Copy link
Collaborator

https://encrypted.google.com/search?q=site%3Auserscripts.org%2Fscripts%2Freview+%22run-at+document-start%22

Lots of existing examples. For the vast majority though, it's either tough to understand what they're intended to do, or how they would ever work. I'd like to spend some time investigating them, though, and confirm that at least some work as intended (both somewhere existing, and in GM with this change).

@qufighter
Copy link
Contributor Author

I was also thinking about adding some more stuff to that original test script. I think the best use case is for preventing flicker (especially when moving flash elements in DOM which can result in video restarting). The first example here should prevent flicker by displaying the body only after everything is ready, however it's not heavily tested: http://wiki.greasespot.net/Talk:Metadata_Block

I am trying to make samples that are backwards compatible as possible. I have tried to interact with nodes during the DOMNodeInserted event but recall only been able to make changes to the node at that time using innerHTML. However it appears appending to the node is very possible. http://userscripts.org/scripts/review/46560

@Martii
Copy link
Contributor

Martii commented Jul 19, 2011

Unit Test: http://userscripts.org/scripts/show/78045

This appears to stop script execution

...
// @run-at document-foobar
...

This STR appears to have issues

  1. ...
    // Unit Test metadata block installed with this
    //
    // @run-at document-start
    //
    // Appears to work okay
    ...
  2. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    // @run-at document-end
    //
    // Appears to work okay
    ...
  3. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    // @run-at document-start
    //
    // Appears to fail on soft and hard page refresh
    ...
  4. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    // @run-at document-end
    //
    // Appears to work again on soft and hard page refresh
    ...
  5. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    // @run-at
    //
    // Appears to work
    ...
  6. ...
    // Unit Test metadata block edited/insert mode with this
    //
    // @run-at
    // @run-at document-start
    //
    // Appears to stop script execution
    ...
  7. ...
    // Unit Test metadata block edited/insert mode with this
    //
    // @run-at
    // @run-at document-start
    // @run-at document-end
    //
    // Appears to work okay
    ...
  8. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    // @run-at document-foo
    // @run-at document-start
    // @run-at document-end
    //
    // Appears to work okay
    ...
  9. ...
    // Unit Test metadata block edited/overwrite mode with this
    //
    //@run-at document-end
    //
    // Appears to work okay (e.g. no @run-at)
    ...
Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20100101 Firefox/5.0
Greasemonkey arantius@e4f5949

@arantius
Copy link
Collaborator

Looking back at the examples from the search I linked above, I see:

  1. Document Start Test
    Simple working PoC test code.
  2. Content Concealer
    Has no reason to need to run at document-start. Would appear (document.body.appendChild) to fail with it. Fails installed in Chrome.
  3. Greasemonkey Emulation
    I don't understand how it works. It just seems to define a bunch of anon-function-local variables. Doesn't seem to need document-start, but rather to be copy/pasted into another script?
  4. Demonoid Ad Remover
    Would seem to break at document-start. None of the nodes it looks for will exist. Has no apparent effect in Chrome.
  5. EZProxy Forwarder
    Seems the perfect use case for document-start, just does a location.href =, so earlier is better. Apparently works in Chrome, in dev GM it is installed as document-end. (Source appears to actually be "// @run-at document-start" (note trailing whitespace).
  6. TA Page Titles
    ??? I don't know where it's supposed to run, seems to be more dom-is-empty failure.
  7. zapEmbedsRefer
    Does nothing but document.addEventListener('load' ...). Pointless to run at -start.
  8. Simple Facebook
    Looks like it properly deals with document-start, by waiting for <head> to exist. Does appear to work in Chrome. In dev GM: not when pressing reload?! (loadType filtering too aggressive?)
  9. FlashBlock
    Appears to work in Chrome, fail in dev GM. Probably because of the hackish setTimeout() calls rather than actually detecting the right time to run? Worth looking into.
  10. defBackgroundUp
    Does nothing but document.addEventListener('load' ...). Pointless to run at -start.
  11. MiiTube
    Just does a document.getElementById() which will fail at -start; confirmed fails in Chrome.
  12. Error Handler
    Just (very conditionally) does document.getElementsByTagName('html')[0].innerHTML = which should fail at -start; untested because I don't want to go looking for a page that matches its conditions.
  13. cho-un-name
    Does a lot of DOM access that would appear to fail at -start no matter what.
  14. Garrysmod Wiki Full Width
    Inline GM_addStyle will fail at -start. Confirmed failure in Chrome.
  15. zapEmbeds
    Does nothing but document.addEventListener('load' ...). Pointless to run at -start.
  16. Remove All Facebook Ads
    Dunno, not a facebook user. Should work if document.addEventListener("DOMNodeInserted", ParsePage, true); behaves as expected by the author.
  17. TubeMirror
    Lots of inline DOM modification will fail regardless. Fails in Chrome.
  18. zapIframes
    Does nothing but document.addEventListener('load' ...). Pointless to run at -start.
  19. draugiem.lv Supercharged
    Inline document.body.appendChild(script); will fail regardless.
  20. Google Disable Auto Focus
    Inline document.body.setAttribute(...) will fail regardless. Confirmed failure in Chrome and Scriptish.

So of the first twenty I see:

  • 4 pointless -start runs (just waiting for a later event)
  • 6 Tested failing in Chrome
  • 4 that work properly in Chrome

And those that work generally also do in dev GM as well; the whitespace parse failure and the reload issue are all I found.

arantius added a commit to arantius/greasemonkey that referenced this issue Jul 20, 2011
arantius added a commit to arantius/greasemonkey that referenced this issue Jul 20, 2011
arantius added a commit to arantius/greasemonkey that referenced this issue Jul 20, 2011
@qufighter
Copy link
Contributor Author

Bug: Doc-Start Scripts are firing during window.location.hash changes
http://www.mozilla.org/projects/#labs

I'm working on flicker prevention proof of concept, this seems to work so far... (does not work well in ff5)

if(document.getElementsByTagName('html')[0])document.getElementsByTagName('html')[0].style.display="none";
document.addEventListener('DOMContentLoaded', function(){
  //page has fully loaded, run DOM interactive scripts here
  document.body.insertBefore(document.createTextNode('Code that executes at DOMContentLoaded'),document.body.firstChild);
  document.body.appendChild(document.createTextNode('Code that executes at DOMContentLoaded'));

  //finally, display the modifications
  document.getElementsByTagName('html')[0].style.display="block";
}, true);

EDIT: Install Link: http://userscripts.org/scripts/show/107491

for some lucky reason document.getElementsByTagName('html')[0] always exists! (ff 3.6)
The more elegant way I think involves modifying default style sheets, or getting GM_addStyle working

I was specifically looking at document.styleSheets, from userscript I don't know how to get anything into that list, which doesn't even exist yet at doc-start time.

@sizzlemctwizzle
Copy link
Contributor

16. Remove All Facebook Ads Dunno, not a facebook user. Should work if document.addEventListener("DOMNodeInserted", ParsePage, true); behaves as expected by the author.
That ParsePage function (which contains a bunch of xpath) will run every single time a node is inserted. That's assuming it doesn't cause the script to fail when document.body is null.

I have a similar script that hides Facebook ads. I use a DOMNodeInserted listener, and then check the tagName of the related node. Once I get the HEAD node, I insert a STYLE node and then remove the listener. In my previous tests there was no noticeable flicker with this method.

@qufighter
Copy link
Contributor Author

During the no-flicker PoC above I ran into many strange DOMNodeInserted, many cases HEAD was not being inserted until after DOMContentLoaded fired, and sometimes after BODY was inserted, defeating the purpose completely. I found this mileage varies per website. Also I found that there was still flicker since DOMNodeInserted fires after the node is inserted, that I could not hide anything added by DOMNodeInserted inside DOMNodeInserted without flicker, and that DOMContentLoaded often fired first anyway.

This is why I think it would be nice to have style rules applied before HEAD exists for attaching styles, I found flicker existed when attempting to do this, and HEAD did not always exist until after DOMContentLoaded. There must be some way to attach css rules directly to the document before anything else.

@arantius
Copy link
Collaborator

Bug: Doc-Start Scripts are firing during window.location.hash changes

Urk, yep. That's a bug. But does present an interesting opportunity: @run-at hash-change anyone?

arantius added a commit to arantius/greasemonkey that referenced this issue Jul 21, 2011
@qufighter
Copy link
Contributor Author

@run-at hash-change intrigues me, it makes sense given the number of AJAX apps today, 3.6+ finally has a listener so I guess people could do it themselves https://developer.mozilla.org/en/DOM/window.onhashchange

We should discuss what we are going to do about execution sort order in script management, doc start scripts ought to be grouped at the top, at least in theory, and have some sort limitations. Do you think we should add a label that says "document-start" next to these scripts somewhere?

@qufighter qufighter reopened this Jul 21, 2011
@arantius
Copy link
Collaborator

We should discuss what we are going to do about execution sort order

This is #1339

@arantius
Copy link
Collaborator

As of the latest commit just above, everything seems to work quite well, but not frames -- document-start scripts never run for them at all (because they're not a tab, and the tab is what the listener is observing).

@qufighter
Copy link
Contributor Author

I thought about iFrames before, it would be tricky to listen to frame location change events. We could look at nodeInserted events and listen for iframe and check the src attribute but then it's a bit too late, chances are the document already started loading. There is also the method of listening that erikvold mentioned which I'd assume works on iframes too.

I guess the question could be, should document-start scripts be executed on iframes at the normal document-end time rather than not at all. I still think the need for document-start scrips on iframes is quite rare.

Arguably one could easily hide the iframe until modifications were performed, especially if one could use
GM_addStyle("iframe{display:none;}");

I suggest GM_addStyle should find some way to work without HEAD element, at document-start time. This would make using doc-start for flicker prevention easy, without having to hide the entire page.

@arantius
Copy link
Collaborator

I'm pushing this into master, it appears to work well enough to move forward, and get it in the nightlies for more testing. To help me and everyone else keep track of things, please report new issues if you do indeed discover something that appears to be wrong.

@arantius
Copy link
Collaborator

I've got #1379 in for tracking the top/frames run target issue.

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

No branches or pull requests

7 participants