1785314 - After deleting a contact, first contact in list gets selected and search box is focused

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[Thomas D. (:thomas8)]

102.1.2 (64-bit), Win10 (same on Daily 105.0a1 (2022-08-16) (64-bit))
This could be considered a regression (the behaviour has changed and is less useful than before, although we may want another behaviour compared to TB 91).

Henry, could this be caused by Bug 1775245? Would this be fixed by one of the bugs on record, e.g. Bug 1752532 (in which case this should depend on that)?
Maybe Alice can identify a regression range after we get a comment from Henry.

STR

• local AB with 4+ contacts
• select contact #3
• press Del key, confirm deletion
• observe focus and selection

Actual

• first contact gets selected - why? This isn't helpful, in fact, it's error-prone especially in combination with next, because now I have a selected contact in the list which I never selected.
• focus ends up in search box - not totally bad, but why? (Or did I suggest this myself??) makes deleting other single contacts from the same list harder, and is a bit jumpy/unexpected.

Expected

• not totally sure, but probably something along the lines of what Henry described on https://phabricator.services.mozilla.com/D151390:

  When deleting a focused and selected item, we try and select the next item that was not deleted, or (if there's no next item) the last item of the list. This prefers keeping the selection at the same position.

• probably no need to jump focus into search box

Note:

• TB 91 used to focus, but not select the first item in the list after deleting an item (which imo was better than now).
  Maybe this was to prevent accidentally deleting more addresses than intended by holding Del a little to long when we didn't have a confirmation for deleting contacts yet. But now with confirmation, Henry's proposed behaviour will probably feel more natural while still being relatively safe against errors.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=577ac8013efd32028f613bdacfee88524b82262b&tochange=c3d2b13c7e8fae994bb72e9e2c52cffbc3b820e4
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0cf5c85ddd8483d622bdf5ba6cf7fdb5db6b90a0&tochange=c2f2df823a0774ede2d61a54597a70f43b790389

Suspect: Bug 1757860

Comment 2

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Thomas D. (:thomas8) from comment #0)

Henry, could this be caused by Bug 1775245? Would this be fixed by one of the bugs on record, e.g. Bug 1752532 (in which case this should depend on that)?

Bug 1775245 only effected "tree-listbox", but the contact list uses "tree-view-listbox". Bug 1752532 would switch the element from using nsTreeSelection to SelectionWidgetController, which should handle re-assigning focus and selection after deleting. But since this is likely a regression from Bug 1757860 it could probably be handled before.

Comment 3

[u695164]

Do you have an idea what the behavior should be with the deletion of multiple selected contacts with contacts in between?
Should it be the next contact of the last selected chosen?

Comment 4

[Thomas D. (:thomas8)]

(In reply to Nicolai Kasper from comment #3)

Do you have an idea what the behavior should be with the deletion of multiple selected contacts with contacts in between?
Should it be the next contact of the last selected chosen?

Good question. I think the answer is yes. Note that focus can be anywhere (even outside the selection), so probably we shouldn't rely on that.
In favor of your suggested solution, it could be argued that when user bothered to select individual entries, he may be done with the parts of the list included in that selection, and proceeding with the next item outside the selection (if such exists) may make sense.

Comment 5

[u695164]

Attachment: Bug 1785314 - Selection of the next contact after deleting contacts for the address book. r=darktrojan

Comment 6

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1785314 - Fix focus and cards list selection when a contact is deleted. r=aleca

This sets the list's current index back to what it was before items were deleted.
Without this patch the current index becomes -1 if the selected row is removed, and then the first item is automatically selected when the list gains focus.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2346cf4b98ce
Fix focus and cards list selection when a contact is deleted. r=aleca

Comment 8

[Geoff Lankow (:darktrojan)]

Comment on attachment 9297066 [details]
Bug 1785314 - Fix focus and cards list selection when a contact is deleted. r=aleca

[Approval Request Comment]
Regression caused by (bug #): new address book
User impact if declined: selection jumps somewhere unexpected when the selected item is deleted
Testing completed (on c-c, etc.): in 107b1
Risk to taking this patch (and alternatives if risky): low

Comment 9

[Wayne Mery (:wsmwk)]

Comment on attachment 9297066 [details]
Bug 1785314 - Fix focus and cards list selection when a contact is deleted. r=aleca

[Triage Comment]
Approved for esr102

Comment 10

[Rob Lemley [:rjl]]

The changes to browser_edit_card.js build upon changes made in bug 1774702, which is not on c-esr102 (likely due to the new strings).

Options, uplift bug 1774702 - 7/10 top 10 have the string, 29/67 overall. Not great.
Or uplift this bug without the test changes, or at least without the changes that have to deal with the delete button added in 1774702.

Comment 11

[Alessandro Castellani [:aleca]]

Or uplift this bug without the test changes, or at least without the changes that have to deal with the delete button added in 1774702.

Yeah, I'm leaning towards this option as adding too many untranslated strings this late into ESR is not great.

Comment 12

[Rob Lemley [:rjl]]

Attachment: 1785314_esr102.patch

[Triage Comment]
This is the same patch, minus the test changes related to test_remove_button() from bug 1774702. There are minor differences in the chunk contexts compared to the original.

Comment 13

[Rob Lemley [:rjl]]

Thunderbird 102.4.1:
https://hg.mozilla.org/releases/comm-esr102/rev/a5bff40427ad