Skip to content

Commit

Permalink
Fix Tab while there's a selection not Tabbing away
Browse files Browse the repository at this point in the history
By "Tabbing away" I mean that if something is focused, and you hit Tab,
you expect focus to jump to the next thing in tab order.

Normally that happens in MathQuill editable fields whenever the cursor
is in the root block/at the top level. But, a bug was intro'd by mathquill#294
and reported in mathquill#413 that if there's a selection, then even if you're in
the root block, when you hit Tab, focus wouldn't actually jump to the
next thing. Instead, the math field would look blurred but the hidden
textarea would "steal back" focus, and you could type in the seemingly-
blurred MathQuill editable field, see GIF: https://git.io/v2UGL
(The "Show Textareas" button in the top-right corner of test/visual.html
 was used to show the normally hidden textareas.)

Note that this bug also affected API clients who in a handler like
`upOutOf` that's called synchronously on `keydown`, blur the textarea
(for example, by focusing something above the math field), which is
exactly what mathquill#355 was supposed to fix. Turns out the test case in mathquill#355
isn't representative, the fix only seemed to work because for
jQuery ≤1.8.3 (our tests use 1.7.2), when you call jQuery's `.blur()`
method, the `blur` event handlers are called while the hidden textarea
is still focused, so the `textarea[0].select()` doesn't "steal back"
focus. However, when Tabbing away, or when the `upOutOf` or whatever
handler focuses something else, or when you call jQuery ≥1.9.0's
`.blur()` method, or (luckily for our tests) when you call the DOM's
"native" `.blur()` method, the hidden textarea is already blurred, so
the `textarea[0].select()` "steals back" focus, hence bug and fix.

Isolated test case demonstrating this for jQuery 1.8.3 vs jQuery 1.9.0
vs the DOM's "native" `.blur()` method:
http://jsbin.com/kepaxo/5/edit?console,output
  • Loading branch information
laughinghan committed Feb 20, 2016
1 parent 3981b19 commit ba1efa4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/services/saneKeyboardEvents.util.js
Expand Up @@ -109,7 +109,7 @@ var saneKeyboardEvents = (function() {
clearTimeout(timeoutId);
timeoutId = setTimeout(checker);
}
target.bind('keydown keypress input keyup focusout paste', function() { checkTextarea(); });
target.bind('keydown keypress input keyup focusout paste', function(e) { checkTextarea(e); });


// -*- public methods -*- //
Expand Down Expand Up @@ -148,8 +148,10 @@ var saneKeyboardEvents = (function() {
keydown = e;
keypress = null;

if (shouldBeSelected) checkTextareaFor(function() {
if (textarea[0].select) textarea[0].select(); // re-select textarea in case it's an unrecognized
if (shouldBeSelected) checkTextareaFor(function(e) {
if (!(e && e.type === 'focusout') && textarea[0].select) {
textarea[0].select(); // re-select textarea in case it's an unrecognized
}
checkTextarea = noop; // key that clears the selection, then never
clearTimeout(timeoutId); // again, 'cos next thing might be blur
});
Expand Down
53 changes: 53 additions & 0 deletions test/unit/focusBlur.test.js
@@ -0,0 +1,53 @@
suite('focusBlur', function() {
suite('handlers can shift focus away', function() {
var mq, mq2, wasUpOutOfCalled;
setup(function() {
mq = MQ.MathField($('<span></span>').appendTo('#mock')[0], {
handlers: {
upOutOf: function() {
wasUpOutOfCalled = true;
mq2.focus();
}
}
});
mq2 = MQ.MathField($('<span></span>').appendTo('#mock')[0]);
wasUpOutOfCalled = false;
});
teardown(function() {
$(mq.el()).add(mq2.el()).remove();
});

function triggerUpOutOf(mq) {
$(mq.el()).find('textarea').trigger(jQuery.Event('keydown', { which: 38 }));
assert.ok(wasUpOutOfCalled);
}
function assertHasFocus(mq, name) {
assert.ok($(mq.el()).find('textarea').is(':focus'), name + ' has focus');
}

test('normally', function() {
mq.focus();
assertHasFocus(mq, 'mq');

triggerUpOutOf(mq);
assertHasFocus(mq2, 'mq2');
});

test('even if there\'s a selection', function(done) {
mq.focus();
assertHasFocus(mq, 'mq');

mq.typedText('asdf');
assert.equal(mq.latex(), 'asdf');

mq.keystroke('Shift-Left');
setTimeout(function() {
assert.equal($(mq.el()).find('textarea').val(), 'f');

triggerUpOutOf(mq);
assertHasFocus(mq2, 'mq2');
done();
});
});
});
});
2 changes: 1 addition & 1 deletion test/unit/saneKeyboardEvents.test.js
Expand Up @@ -166,7 +166,7 @@ suite('key', function() {
var shim = saneKeyboardEvents(el, {
keystroke: function(key) {
assert.equal(key, 'Left');
el.blur();
el[0].blur();
}
});

Expand Down

0 comments on commit ba1efa4

Please sign in to comment.