Skip to content
This repository

GM_xmlhttpRequest really needs a synchronous mode. #1269

Closed
wants to merge 7 commits into from

4 participants

meh. Erik Vold Naji arantius
meh.
meh commented January 27, 2011

Shouldn't be a problem to merge to master.

Or reimplement it in some other way, this was just a fast hack to get something I needed working.

Thanks.

meh.
meh commented January 28, 2011

Eh, obvious security bugs, didn't read the setupRequestEvent properly.

There shouldn't be security issues now.

Erik Vold

why is a synchronous mode needed? it's a terrible thing to do..

meh.
meh commented April 16, 2011

Because asynchronous requests aren't always the best way, and I don't see why not giving the developer the chance to use synchronous requests.

Erik Vold

Because asynchronous requests aren't always the best way

when is it not?

meh.
meh commented April 16, 2011

In my case I had to try multiple lyrics website one after another if the previous failed, and setting up it to work with asynchronous requests would be an useless overengineering when there's synchronous mode.

Erik Vold

In my case I had to try multiple lyrics website one after another if the previous failed, and setting up it to work with asynchronous requests would be an useless overengineering when there's synchronous mode.

It's not useless.. synchronous xhr locks the ui, asynchronous requests do not lock the ui..

meh.
meh commented April 17, 2011

It doesn't lock the UI for me.

And once again, leave the choice to the developer.

Naji

That right, the possibility to have a synchronous mode for the function GM_xmlhttprequest will really help the developers. In some case, it can be very usefull.

That why as meh said, leave the choice to the developer is a great solution.

Thanks.

meh. Merge remote-tracking branch 'upstream/master'
Conflicts:
	content/xmlhttprequester.js
ab11a1b
arantius arantius referenced this pull request from a commit in arantius/greasemonkey August 05, 2011
arantius Merge commit '51e17b7c703c75c074811a57e230690ef2fc077d'
Conflicts:
	content/xmlhttprequester.js

Fixes: #1269
0478edb
arantius arantius closed this August 05, 2011
arantius
Collaborator

In at least some circumstances, this is currently failing with
Error: Permission denied for <http://arantius.com> to get property Object.status
Source File: file:///.../gm_scripts/sync_gm_xhr_test/sync_gm_xhr_test.user.js
Line: 14

Though I'm sure I saw it work at least once, also.
Currently testing in: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110628 Ubuntu/10.04 (lucid) Firefox/3.6.18

Test case: https://gist.github.com/1134362

arantius arantius reopened this August 09, 2011
arantius
Collaborator

Working on Mozilla/5.0 (X11; Linux i686 on x86_64; rv:7.0a2) Gecko/20110809 Firefox/7.0a2.

arantius
Collaborator

Also working: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1.

So, failing in 3 only, working in 4+.

arantius arantius closed this pull request from a commit August 10, 2011
arantius Don't use getters for synchronous request return values.
The request was synchronous, so the values are available now anyway.  Works with Firefox 3.6 as well as 4+.

Fixes #1269
c12141c
arantius arantius closed this in c12141c August 10, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 13 additions and 4 deletions. Show diff stats Hide diff stats

  1. 17  content/xmlhttprequester.js
17  content/xmlhttprequester.js
@@ -40,9 +40,14 @@ GM_xmlhttpRequester.prototype.contentStartRequest = function(details) {
40 40
   }
41 41
 
42 42
   return {
43  
-    abort: function() {
44  
-      req.abort();
45  
-    }
  43
+    get responseText()    req.responseText,
  44
+    get responseHeaders() req.getAllResponseHeaders(),
  45
+    get readyState()      req.readyState,
  46
+    get status()          req.status,
  47
+    get statusText()      req.statusText,
  48
+    get finalUrl()        req.channel.URI.spec,
  49
+
  50
+    abort: function () req.abort()
46 51
   };
47 52
 };
48 53
 
@@ -59,7 +64,7 @@ function(safeUrl, details, req) {
59 64
 
60 65
   req.mozBackgroundRequest = !!details.mozBackgroundRequest;
61 66
 
62  
-  req.open(details.method, safeUrl, true, details.user || "", details.password || "");
  67
+  req.open(details.method, safeUrl, !details.synchronous, details.user || "", details.password || "");
63 68
 
64 69
   if (details.overrideMimeType) {
65 70
     req.overrideMimeType(details.overrideMimeType);
@@ -81,6 +86,10 @@ function(safeUrl, details, req) {
81 86
   } else {
82 87
     req.send(body);
83 88
   }
  89
+
  90
+  GM_log("< GM_xmlhttpRequest.chromeStartRequest");
  91
+
  92
+  return req;
84 93
 };
85 94
 
86 95
 // sets the "Referer" HTTP header for this GM_XHR request.
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.