Skip to content

Loading…

Intercept <esc> avoid closing websocket on Firefox #1032

Closed
wants to merge 1 commit into from

4 participants

@Carreau

closes #1031

To rephrase what on #1031, default action of <esc> on Firefox is to stop the page loading, and it also closes the websocket conexion.
so I intercept it a notebook level and call event.preventDefault()

@takluyver takluyver commented on the diff
IPython/frontend/html/notebook/static/js/notebook.js
@@ -57,6 +57,11 @@ var IPython = (function (IPython) {
$(document).keydown(function (event) {
// console.log(event);
if (that.read_only) return;
@takluyver IPython member

Do we need to catch it before this line? I don't know if read_only means passive observer (using websockets) or static view (not using websockets).

@Carreau
Carreau added a note

In my test, it is static-view, and the websocket doen't close when pressing <esc> even without this patch when read-only, so it doesn't really matters...

IMHO, I prefer to have it after if(that.read_only) because it kind of separate read-only logic from key handeling logic.

@minrk IPython member
minrk added a note

As it is currently defined, read-only mode establishes no websocket connections.

That's part of what read-only means - no connections to the kernel (all websocket connections are for kernel communication).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@minrk minrk commented on the diff
IPython/frontend/html/notebook/static/js/notebook.js
@@ -57,6 +57,11 @@ var IPython = (function (IPython) {
$(document).keydown(function (event) {
// console.log(event);
if (that.read_only) return;
+ if (event.which === 27) {
+ // Intercept escape at highest level to avoid closing
+ // websocket connexionwith firefox
@minrk IPython member
minrk added a note

typo 'connexionwith' -> 'connection with'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez fperez added a commit that closed this pull request
@Carreau Carreau Intercept <esc> avoid closing websocket on Firefox
Closes #1031; closes #1032 (rebased and fixed tiny typo)
a329ff0
@fperez fperez closed this in a329ff0
@fperez
IPython member

Thanks! I confirmed that it fixes the problem on ffox and fixed the tiny typo. Merged (with a rebase to avoid recursive merge) and pushed. Thanks everyone for the review too...

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request
@Carreau Carreau Intercept <esc> avoid closing websocket on Firefox
Closes #1031; closes #1032 (rebased and fixed tiny typo)
ec8b56c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 23, 2011
  1. @Carreau
Showing with 5 additions and 0 deletions.
  1. +5 −0 IPython/frontend/html/notebook/static/js/notebook.js
View
5 IPython/frontend/html/notebook/static/js/notebook.js
@@ -57,6 +57,11 @@ var IPython = (function (IPython) {
$(document).keydown(function (event) {
// console.log(event);
if (that.read_only) return;
@takluyver IPython member

Do we need to catch it before this line? I don't know if read_only means passive observer (using websockets) or static view (not using websockets).

@Carreau
Carreau added a note

In my test, it is static-view, and the websocket doen't close when pressing <esc> even without this patch when read-only, so it doesn't really matters...

IMHO, I prefer to have it after if(that.read_only) because it kind of separate read-only logic from key handeling logic.

@minrk IPython member
minrk added a note

As it is currently defined, read-only mode establishes no websocket connections.

That's part of what read-only means - no connections to the kernel (all websocket connections are for kernel communication).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (event.which === 27) {
+ // Intercept escape at highest level to avoid closing
+ // websocket connexionwith firefox
@minrk IPython member
minrk added a note

typo 'connexionwith' -> 'connection with'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ event.preventDefault();
+ }
if (event.which === 38) {
var cell = that.selected_cell();
if (cell.at_top()) {
Something went wrong with that request. Please try again.