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

GM_xmlhttpRequest progress event #1081

Closed
ghost opened this issue Feb 21, 2010 · 13 comments
Closed

GM_xmlhttpRequest progress event #1081

ghost opened this issue Feb 21, 2010 · 13 comments
Milestone

Comments

@ghost
Copy link

ghost commented Feb 21, 2010

Now that GM_xmlhttpRequest supports the sendasbinary method, could you please make the "progress" event accessible in the GM_xmlhttpRequest?

In a regular xmlhttpRequest, it's accessed like this:
xhr.upload.addEventListener("progress", function(e) {...

https://developer.mozilla.org/en/XMLHttpRequest#status
http://hacks.mozilla.org/2009/12/uploading-files-with-xmlhttprequest/

@erikvold
Copy link
Contributor

erikvold commented Apr 6, 2010

I wrote a patch here, I'm not sure it's safe but I think that it is.

@Ventero
Copy link
Contributor

Ventero commented Apr 6, 2010

I get an access denied-error when trying to access any of xhr.upload's properties (with Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3, GM 0.8.6RC1 and your patch of course).
Also not sure if this is relevant:

Note: You need to add the event listeners before calling open() on the request. Otherwise the progress events will not fire.
(taken from MDC). If that's true for upload events, your approach won't work for the upload-progress-event, since xhr.open and xhr.send are called before rtnReq is returned.

@erikvold
Copy link
Contributor

erikvold commented Apr 6, 2010

I get an access denied-error when trying to access any of xhr.upload's properties

Thanks for testing that Venero, I hadn't gotten around to that yet, would you mind making a gist of the test you used?.

Note: You need to add the event listeners before calling open() on the request. Otherwise the progress events will not fire.

(taken from MDC).

hmm.. we could accept new properties for the details object provided, which takes upload event listeners like it does onload/onerror, the only question would be what to call them.. maybe upload.onprogress? so the code would look like:

GM_xmlhttpRequest({
  method: "GET",
  url: "http://www.example.com/",
  onload: function(response) {
    ...
  },
  upload: {
    onprogress: function(e){...},
    onload: function(e){...},
    onerror: function(e){...},
    onabort: function(e){...}
  }
});

@Ventero
Copy link
Contributor

Ventero commented Apr 6, 2010

The gist is here.

hmm.. we could accept new properties for the details object provided, which takes upload event listeners like it does onload/onerror

Sounds like a good idea to me, and I guess the event names should be consistent with the request-events, so I think we should use onload, onprogress etc.
I guess a function similar to GM_xhr.setupRequestEvent could be used to set up the upload events. Since there's a few event-properties that can't be accessed (i.e. explicitOriginalTarget) without throwing an exception, I'd say the function should create a new object which will be passed to the callback instead of the original event, just like setupRequestEvent does.
The properties that object should have can be seen here.

@sizzlemctwizzle
Copy link
Contributor

the only question would be what to call them..
MDC: Progress events exist for both download and upload transfers.
I would go with something like:
GM_xmlhttpRequest({
  method: "GET",
  url: "http://www.example.com/",
  onload: function(response) {
    ...
  },
  uploadProgress: {
      onprogress: function(e){...},
      onload: function(e){...},
      onerror: function(e){...},
      onabort: function(e){...}
  }
});
I guess a function similar to GM_xhr.setupRequestEvent could be used to set up the upload events.
I say we modify GM_xhr.chromeStartRequest to attach all the events(both existing and the new upload/download progress events) with GM_xhr.setupRequestEvent.

@erikvold
Copy link
Contributor

erikvold commented Apr 6, 2010

I'd say the function should create a new object which will be passed to the callback instead of the original event, just like setupRequestEvent does.

Agreed

@erikvold
Copy link
Contributor

erikvold commented Apr 6, 2010

GM_xmlhttpRequest({
  method: "GET",
  url: "http://www.example.com/",
  onload: function(response) {
    ...
  },
  progress: {
    upload: {
      onprogress: function(e){...},
      onload: function(e){...},
      onerror: function(e){...},
      onabort: function(e){...}
    },
    download: {
      onprogress: function(e){...},
      onload: function(e){...},
      onerror: function(e){...},
      onabort: function(e){...}
    }
  }
});

Wouldn't details.onload and details.progress.download.onload listen to the same event?

@erikvold
Copy link
Contributor

erikvold commented Apr 7, 2010

I think adding onprogress, in:


GM_xmlhttpRequest({
  method: "GET",
  url: "http://www.example.com/",
  onload: function(response) {
    ...
  },
  onprogress: function(e) {
    ...
  }
});

Should be part of this issue as well.

@sizzlemctwizzle
Copy link
Contributor

Wouldn't details.onload and details.progress.download.onload listen to the same event?
I read the documentation more and apparently the download events are already called on the regular events so there's no need to implement them. Instead I'm going to use a uploadProgress object for the upload events.
I think adding onprogress, in:
Yeah I'm adding that in and onabort too.

@sizzlemctwizzle
Copy link
Contributor

Okay, I've got a patch for this issue in this commit: http://github.com/sizzlemctwizzle/greasemonkey/commit/4cd996cc1d8b6f480751d1830ee3bfa559060c8e

Here is a script to test the upload onload event(the other events are harder to test): http://gmconfig.googlecode.com/svn/trunk/test_all_events.user.js

@sizzlemctwizzle
Copy link
Contributor

I updated the branch for this issue.

@arantius
Copy link
Collaborator

arantius commented Aug 5, 2011

I've merged sizzle's branch. A test script (those linked above now give 403): https://gist.github.com/1127807

Produces:

>>> GM_xhr test starting.
<<< onreadystatechange!
<<< onreadystatechange!
<<< onreadystatechange!
<<< onreadystatechange!
<<< onprogress!
<<< onprogress!
<<< onreadystatechange!
<<< onprogress!
<<< onreadystatechange!
<<< onprogress!
<<< onreadystatechange!
<<< onprogress!
<<< onreadystatechange!
<<< onprogress!
<<< onreadystatechange!
<<< onreadystatechange!
<<< onload!

Where the linked script does a bunch of print/flush/sleep things to simulate a long incremental download. And a manual change to call .abort() on the request with a timeout produced the expected results.

I haven't tested upload listeners, but it seems like they'll work if the rest do.

@arantius
Copy link
Collaborator

arantius commented Aug 5, 2011

Fixed by 675c0c1 .

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

4 participants