1679113 - Search from autocomplete input on toolbar opens three gloda tabs

Status: RESOLVED FIXED

(Thunderbird :: Search, defect)

Description

[Geoff Lankow (:darktrojan)]

STR: In the toolbar search widget, type something you know will produce results. Click on one of the suggestions in the drop-down. Three tabs are opened instead of one.

Comment 1

[Wayne Mery (:wsmwk)]

I see this with 84.0b1.
Alice, can you get a regression range?

Comment 2

[Alice0775 White]

What is "toolbar search widget"?

never mind, I see Ctrl+k to focus the search field

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=06424ba4ea534eee5cf442b720daa0fb7757321e&tochange=2efdb12bede31bf730d98216ac68260e26d4ee9e
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=600f47bbfeb2b8dd8feb52dc9b0df0c72e01da9e&tochange=1581160e62e639714efe31ccf89e1f7f3c07d202

Comment 5

[Wayne Mery (:wsmwk)]

Thanks Alice. That puts a pretty fine point on the cause

Comment 7

[Lasana Murray]

We have three autocomplete widgets in the messenger.xhtml for global search at the following selectors:
.remote-gloda-search, #IMSearchInput and #searchInput. All three hook into the "autocomplete-did-enter-text" nsIObserver event
to handle clicks on the autocomplete suggestions. The underlying C++ code does not seem to provide the original element as the subject for these events so the current code checks subject.popupElement value to determine which gloda-autocomplete-input to handle the event.

Unfortunately, the 3 autocompletes are all using the same popupElement so the checks will always be true.

Comment 8

[Lasana Murray]

Attachment: bug1679113.patch

This changes the check to use document.activeElement instead of checking the popup element. Also adds some tests to make sure this action opens one tab however it will fail for the chat tab because of the issue in bug 1680587.

Comment 9

[Lasana Murray]

Requesting review from Alessandro because of the conversion work in bug 1542720. Bug 1542720 comment 24 is related.

Comment 10

[Alessandro Castellani [:aleca]]

Comment on attachment 9191273 [details] [diff] [review]
bug1679113.patch

Review of attachment 9191273 [details] [diff] [review]:
-----------------------------------------------------------------

This fixes the issue in the gloda search.
Good stuff in adding a test to cover this area, but unfortunately the test fails locally on macos.

This is the failure log:
FAIL uncaught exception - TypeError: can't access property "hasAttribute", window.gBrowser.selectedBrowser is null at AutocompleteInput/<@chrome://global/content/elements/autocomplete-input.js:70:1
testClickingGlobalSearchResultItemOpensOneTab@chrome://mochitests/content/browser/comm/mail/test/browser/global-search-bar/browser_clickResultItem.js:106:11

Comment 11

[Lasana Murray]

unfortunately the test fails locally on macos

This patch in bug 1680587 fixes this.

Comment 12

[Alessandro Castellani [:aleca]]

Comment on attachment 9191273 [details] [diff] [review]
bug1679113.patch

Review of attachment 9191273 [details] [diff] [review]:
-----------------------------------------------------------------

You're right, bug 1680587 fixes the test failure.
Updating my review.

Comment 13

[Lasana Murray]

Cool! thanks! Just need a resolution for bug 1680587 and this should be fixed.

Comment 14

[Lasana Murray]

Attachment: bug1679113v2.patch

Skipped the chat search test because my patch for bug 1680587 is unlikely to be accepted. Meanwhile this bug has a direct impact on usability.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9191679 [details] [diff] [review]
bug1679113v2.patch

Review of attachment 9191679 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/db6e5c49fa30
Ensure MozGlodaAutocompleteInput is active element before handling "autocomplete-did-enter-text". r=mkmelin