Closed Bug 1644026 Opened 3 years ago Closed 3 years ago

Incremental search in contacts side bar fails to return (some/all) local AB results by swapping them with LDAP results

Categories

(Thunderbird :: Address Book, defect)

defect

Tracking

(thunderbird_esr78 fixed, thunderbird79 fixed)

RESOLVED FIXED
Thunderbird 80.0
Tracking Status
thunderbird_esr78 --- fixed
thunderbird79 --- fixed

People

(Reporter: thomas8, Assigned: darktrojan)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(4 files)

Seen on 79.0a1 (2020-06-07) (64-bit)

I am seeing chaos happening in the results pane of contacts side bar, with local AB results randomly being swapped against LDAP results instantly after being found (see screencast attached to my next comment).

STR (not reduced)

  1. Have some local AB entries in Personal AB:
    Jane Doe <jane@asdf.com>
    John Doe <john@asdf.com>
    John Doe <j@asdf.com> [First name: John Doe (sic), Last name: lastnam (sic), Display Name: John Doe.] - That's what I had for testing, need to reduce that and check if relevant].

  2. Have an LDAP AB set up: Use properties from attached screenshot, it's public. Let's try to give them a bit of privacy nevertheless, looks like real data: filtered screenshot, dis=torted= names in bug text. At the end of Base DN it's dc=edu.

  3. Compose, Contacts Side bar: All Address Books; Search Contacts: Type Doe, slowly, pause after each character, and monitor results pane changes closely.

Actual result:

  • Results are playing hide and seek... (see screencast, next comment):
  • Do initially returns the three local AB results from step 1, then instantly overwrites them with three results from the LDAP-DIR (Cor=do=va; Day, =Do=lores; DelTon=do=)
  • Doe (and Doe,) initially returns three local results, then instantly overwrites only the last local result (John Doe <j@asdf.com>) with an LDAP result (=Do=ell, M.); note that 100 results max is set for LDAP, so =Do=ell is probably > 100, hence correctly not returned before.

Expected result:

  • show all matching local and LDAP results consistently
  • do not hide matching local AB results to replace them with others from LDAP

So here's the screencast showing the bug.
Watch how (some/all) Contacts Sidebar local AB results get replaced with LDAP results instantly after being returned.

Note how only 2 local results are returned in sidebar, but 3 local results for same searchword in recipient autocomplete (both sharing same AB search algorithm). For the video, LDAP autocomplete was off, so correctly no LDAP results on autocomplete.

This snapshot shows missing local AB result for John Doe <j@asdf.com> in Contacts Sidebar (2 local results, 1 LDAP result), vs. recipient autocomplete, where it's shown (3 local results, 1 LDAP result). Now with LDAP autocomplete activated.

There's also a small theme inconsistency: LDAP results have an LDAP globe icon in autocomplete results, but no special icon in AB search results. Richard, maybe we could have a consistent icon for both, e. g. the contact person icon with a small globe? Separate bug if you so wish.

Flags: needinfo?(richard.marti)

If we expose it in the tree we could make it happen here (https://searchfox.org/comm-central/source/mail/themes/shared/mail/abResultsPane.css#13). But I've never seen such a selector.

Flags: needinfo?(richard.marti)

(In reply to Richard Marti (:Paenglab) from comment #3)

If we expose it in the tree we could make it happen here (https://searchfox.org/comm-central/source/mail/themes/shared/mail/abResultsPane.css#13). But I've never seen such a selector.

They are all over, and clearly using various attributes of tree items: https://searchfox.org/comm-central/search?q=-moz-tree-image&path=

Maybe better do discuss that in a separate bug.

It has to be in this tree and it is done through C++. And then we can set the correct selector.

See Also: → 1644084

Enterprise will be especially affected by this chaos in result sets involving LDAP matches.

Geoff, from your experience in Bug 1639750, any insights here?

Flags: needinfo?(geoff)

Yes, I know what's going on (now). The tree isn't getting the message that there is more rows. So when new results arrive we see the same number of rows as before, but sorting happened so which ones we see probably changed.

In this patch I explicitly tell the tree that the row count has changed, instead of relying on the rather hacky way it worked before. Also I just put the search results into the tree as they arrive instead of waiting for them all then sorting.

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(geoff)
Attachment #9160565 - Flags: review?(rob)
Attachment #9160565 - Flags: review?(rob) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/9092f3023333
Fix incremental address book search. r=rjl

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 80.0
Comment on attachment 9160565 [details] [diff] [review]
1644026-incremental-search-1.diff

I want this bad UX to be fixed in 78.1.
Attachment #9160565 - Flags: approval-comm-beta?
Keywords: regression
Attachment #9160565 - Flags: approval-comm-esr78?
Comment on attachment 9160565 [details] [diff] [review]
1644026-incremental-search-1.diff

Approved for 79 beta, to prepare for 78
Attachment #9160565 - Flags: approval-comm-beta? → approval-comm-beta+
Comment on attachment 9160565 [details] [diff] [review]
1644026-incremental-search-1.diff

Approved for esr78.
But let's verify it's fixed on the beta - Thomas can you check?
Flags: needinfo?(bugzilla2007)
Attachment #9160565 - Flags: approval-comm-esr78? → approval-comm-esr78+

(In reply to Wayne Mery (:wsmwk) from comment #13)

But let's verify it's fixed on the beta - Thomas can you check?

Yes, fixed on 79.0b1 (32-bit).

Flags: needinfo?(bugzilla2007)
You need to log in before you can comment on or make changes to this bug.