Commit 65bb2a31 authored by Dan Beam's avatar Dan Beam Committed by Commit Bot

Local NTP, Realbox: fix another suggestion removal focusout bug

I accidentally missed a case (or merge conflicted or something) in which
we should be ignoring focusout when the user deletes the focused item.

This specific case reproduces with these steps:
0) Search for "what are carbs" (and execute search)
1) Type "what" into realbox on NTP afterward
2) Move focus to "what are carbs" result
3) Press Shift+Delete

I've added an assert() to ensure this doesn't happen again, and added a
test. I've also updated a test that was triggering the assert() because
it was differing from what the prod code actually does (there was a
comment admitting that).

Bug: 1002689
Change-Id: Ic534ba725931291c3c327a43ea52d65d27ae51a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868092
Auto-Submit: Dan Beam <dbeam@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707461}
parent 9ed68bfb
......@@ -1076,6 +1076,8 @@ function onAddCustomLinkDone(success) {
/** @param {!DeleteAutocompleteMatchResult} result */
function onDeleteAutocompleteMatch(result) {
assert(matchElBeingDeleted);
if (!result.success) {
matchElBeingDeleted = null;
return;
......@@ -1305,6 +1307,7 @@ function onRealboxWrapperKeydown(e) {
if (key === 'Delete') {
if (e.shiftKey && !e.altKey && !e.ctrlKey && !e.metaKey &&
autocompleteMatches[selected].supportsDeletion) {
matchElBeingDeleted = matchEls[selected];
window.chrome.embeddedSearch.searchBox.deleteAutocompleteMatch(selected);
e.preventDefault();
}
......
......@@ -518,17 +518,10 @@ test.realbox.testUnsupportedDeletion = function() {
});
test.realbox.realboxEl.dispatchEvent(keyEvent);
assertFalse(keyEvent.defaultPrevented);
assertEquals(0, test.realbox.deletedLines.length);
const matchesEl = $(test.realbox.IDS.REALBOX_MATCHES);
assertFalse(matchesEl.classList.contains(test.realbox.CLASSES.REMOVABLE));
// The below 2 statements shouldn't really happen in updated code but isn't
// terrible idea to keep testing for now. This is because SupportsDeletion()
// is now propagated to the client, so we shouldn't allow users (via the UI)
// to attempt to delete non-deletable things.
chrome.embeddedSearch.searchBox.ondeleteautocompletematch(
{success: false, matches: []});
assertEquals(1, $(test.realbox.IDS.REALBOX_MATCHES).children.length);
};
test.realbox.testSupportedDeletion = function() {
......@@ -566,6 +559,14 @@ test.realbox.testSupportedDeletion = function() {
test.realbox.realboxEl.dispatchEvent(shiftDelete);
assertTrue(shiftDelete.defaultPrevented);
// Pretend like the focused match gets removed from DOM when deleted.
matchesEl.children[1].dispatchEvent(new Event('focusout', {bubbles: true}));
// stopAutocomplete() should not be called if the focused match gets removed
// from the DOM (and focus gets dropped). There's explicit code in the
// focusout handler to deal with this case.
assertEquals(0, test.realbox.stops.length);
assertEquals(1, test.realbox.deletedLines.length);
assertEquals(1, test.realbox.deletedLines[0]);
......
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