-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Monkeypatch Tornado 2.1.1 so it works with Google Chrome 16. #948
Conversation
We're just applying manually a fix from Tornado itself, see for details: tornadoweb/tornado#385 tornadoweb/tornado@84d7b458f956727c3b0d6710
|
||
import tornado | ||
|
||
if tornado.version == '2.1.1': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do this check so that it also gets 2.1.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think 2.1.0 doesn't actually work, if I remember right. But perhaps it does? Do you know for sure it does? If so, then yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be working for me with Firefox (I installed tornado just after the 2.1 release). I guess there aren't many people using it, but it's simple enough to do tornado.version_info <= (2,1,1)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the version_info value for 2.1? I'd rather check specifically for 2.1 or 2.1.1: monkeypatches are ugly, I like them as tightly constrained as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our version checks are for 2.1.0, so if it doesn't work, we should update them. version_info reports (2,1,0) for 2.1.0, though since tornado < (2,1,0) doesn't work at all (you can't even import the handlers file where the monkeypatch resides if tornado is pre-2.1 because notebook.__init__
checks for it), you should be able to do a simple <= (2,1,1)
test.
Let's open a low-priority issue when we merge this, to remove it when it's no longer needed, just so it doesn't get lost under the radar. |
Good idea, go ahead. |
OK, I've made the change to using version_info and working with 2.1.{0,1}. Thanks for the review. Anything else, or should I merge? |
Looks good to me |
Monkeypatch Tornado 2.1.{0, 1} so it works with Google Chrome 16. This uses a fix from Tornado itself that's just a protocol number check, no actual protocol handling code is necessary.
Monkeypatch Tornado 2.1.{0, 1} so it works with Google Chrome 16. This uses a fix from Tornado itself that's just a protocol number check, no actual protocol handling code is necessary.
We're just applying manually a fix from Tornado itself, see for
details:
tornadoweb/tornado#385
tornadoweb/tornado@84d7b458f956727c3b0d6710
This isn't sufficient to take care of #934 completely, as we should also put out a proper websocket diagnostic message for any connection message. But it should at least take care of users of the -dev Chrome channel in normal circumstances.
@satra, let us know if this fixes the behavior you saw on Friday; I tested with Chrome 16.... on my linux box and it does fix it for me.