Skip to content

GitLab

T
tangled
Commit c5138fa2 authored by yosin's avatar yosin Committed by Commit bot
Browse files
Options

Keyboard event should not insert a character at selection if selection doesn't have focus 

This patch makes Blink not to insert a character at selection if selection
doesn't have focus to align with other browser's behavior.

The test expectation of "keypress-focus-change.html" is changed for moving focus
on "keydown" event handler. In Firefox, it still insert a character at selection
but Blink and Edge don't. On manual focus moving, Firefox also doesn't insert
character.

To unblock CL[1] as soon as possible, this patch uses ugly test case. Following
patch will fix it.

[1] http://crrev.com/2616623002: Do not send redundant
ViewHostMsg_TextInputStateChanged-message

BUG=89026
TEST=LayoutTests/editing/input/keyboard_event_without_focus.html

Review-Url: https://codereview.chromium.org/2628763003
Cr-Commit-Position: refs/heads/master@{#443189}
parent 2dcda31b
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 4 deletions
third_party/WebKit/LayoutTests/editing/input/keyboard_event_without_focus.html 0 → 100644
View file @ c5138fa2
<!doctype html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<script>
test(() => {
if (!window.eventSender) {
assert_unreached('This test requires eventSender.');
return;
}
assert_selection(
'<input value="x"><input type="checkbox">',
selection => {
const document = selection.document;
const textField = document.querySelector('input');
textField.select();
document.querySelector('input[type=checkbox]').focus();
eventSender.keyDown('a');
// Insert |textField.value| to HTML for verification
textField.appendChild(document.createTextNode(textField.value));
},
'<input value="x">x</input><input type="checkbox">',
'unfocused text field should not insert a charcter by keyboard event');
assert_selection(
'<div contenteditable>|</div><input type="checkbox">',
selection => {
const document = selection.document;
document.querySelector('input[type=checkbox]').focus();
eventSender.keyDown('a');
},
'<div contenteditable>|</div><input type="checkbox">',
'unfocused content editable should not insert a charcter by keyboard event');
}, 'Keyboard event without focus should not insert a character.');
</script>
third_party/WebKit/LayoutTests/fast/events/keypress-focus-change-expected.txt
View file @ c5138fa2
This tests that when the keyPress event handler changes focus to a new form field,
that the key will still be inserted in the original field that was the target of the event.
that the key will not be inserted in the original field that was the target of the event.
Note: Edge will not inserted. Firefox and WebKit will insert.
Test Passed
third_party/WebKit/LayoutTests/fast/events/keypress-focus-change.html
View file @ c5138fa2
......@@ -15,7 +15,7 @@
eventSender.keyDown("2");
eventSender.keyDown("3");
}
if (tf0.value == "1" && tf1.value == "23") {
if (tf0.value == "" && tf1.value == "23") {
res.innerHTML = "Test Passed";
} else {
res.innerHTML = "Test Failed: first field = " + tf0.value + " second field = " + tf1.value;
......@@ -25,7 +25,8 @@
</script>
<body onload="test()">
This tests that when the keyPress event handler changes focus to a new form field,<br>
that the key will still be inserted in the original field that was the target of the event.<br>
that the key will not be inserted in the original field that was the target of the event.<br>
Note: Edge will not inserted. Firefox and WebKit will insert.
<input id="tf0" onkeypress="document.getElementById('fr').contentDocument.getElementById('tf1').focus();">
<iframe id="fr" style="border: solid thin black;" src="resources/keypress-frame.html"></iframe>
<br>
......
third_party/WebKit/Source/core/editing/EditorKeyBindings.cpp
View file @ c5138fa2
......@@ -60,6 +60,18 @@ bool Editor::handleEditingKeyboardEvent(KeyboardEvent* evt) {
if (!behavior().shouldInsertCharacter(*evt) || !canEdit())
return false;
const Element* const focusedElement = m_frame->document()->focusedElement();
if (!focusedElement) {
// We may lose focused element by |command.execute(evt)|.
return false;
}
if (!focusedElement->containsIncludingHostElements(
*m_frame->selection().start().computeContainerNode())) {
// We should not insert text at selection start if selection doesn't have
// focus. See http://crbug.com/89026
return false;
}
// Return true to prevent default action. e.g. Space key scroll.
if (dispatchBeforeInputInsertText(evt->target(), keyEvent->text) !=
DispatchEventResult::NotCanceled)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment