Closed Bug 1732553 Opened 1 year ago Closed 3 months ago

Dragging unselected contact B from the new Address Book drops selected contact A instead (mail.addr_book.useNewAddressBook=true)

Categories

(Thunderbird :: Address Book, defect, P2)

Thunderbird 91

Tracking

(thunderbird_esr102 fixed, thunderbird103 fixed)

RESOLVED FIXED
104 Branch
Tracking Status
thunderbird_esr102 --- fixed
thunderbird103 --- fixed

People

(Reporter: G.Gersdorf, Assigned: darktrojan)

References

(Blocks 1 open bug)

Details

(Keywords: privacy, ux-mode-error, Whiteboard: [STR in comment 7])

Attachments

(5 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

Drag and drop an address from the new addressbook to anywhere.

Actual results:

If no address is currently selected (nothing is shown in the right pane) nothing happens.
If an address is selected (shown in the right pane) that address is dropped.

Expected results:

Should drag and drop the address where the drag started (where the mouse has been clicked) not the currently selected address.

Sorry, maybe it's a translation issue but none of these sentences makes sense to me. Please give more detailed steps.

For example you are dragging an email address or a contact/card?

Flags: needinfo?(G.Gersdorf)

Sorry for my bad english, i try it again:

Open the new addressbook, in the middle pane click on a card (e.g. 'person1'). This card is now highlighted and shown in the right pane.
Now click onto another card (e.g. 'person2') and drag it into the 'To:' field of a new mail window.
I would expect to see 'person2' in the 'To:' field, but 'person1' is shown.

Flags: needinfo?(G.Gersdorf)

Günter, is this what you're trying to do?

Works for me on 91.5.0 (64-bit), Win10 (see video).

Maybe add-on problem? Does the problem still appear when using the following mode?
≡ > Help > Troubleshoot Mode…

Flags: needinfo?(G.Gersdorf)

The video is using the old addressbook. I'm speaking about the new addressbook (mail.addr_book.useNewAddressBook=true)

Flags: needinfo?(G.Gersdorf)

Or maybe this?

  1. Select card 1
  2. Hold Ctrl (Strg) and start dragging card 2 into To field of a composition

Actual = Expected

  • only card 1 is dragged

Imo this is expected. Holding Ctrl means "preserve current selection for the next action". Current selection was card 1 and this was preserved when you started your drag. You did not add card 2 to the selection because that requires Ctrl+click (not Ctrl+drag). As a convenience exception (kind of), unmodified drag of an unselected item will first select that item.

(In reply to Günter Gersdorf from comment #4)

The video is using the old addressbook. I'm speaking about the new addressbook (mail.addr_book.useNewAddressBook=true)

OK, thanks for making that explicit. New address book is not enabled by default in TB 91, so describing your special setting would have made a good first step for us to understand what you're doing. I know you mentioned "the new address book" in your steps, but I read that as "a new address book"...

Confirming for TB 91.5.0 (64-bit), Win10.

STR

  1. Enable New Address Book via pref: mail.addr_book.useNewAddressBook=true
  2. Open New Address Book (Ctrl+Shift+B)
  3. Open a composition window side by side with TB main window which shows AB
  4. Select contact 1 (henry@example.com)
  5. Drag contact 2 (jane@example.com) to To field of composition

Actual

  • contact 1 (henry...) got added

Expected

  • only contact 2 (jane...) was dragged, so it should have been added.
  • to achieve this, contact 2 (jane...) should have been selected when the drag was started on an unselected card without any modifier keys like Ctrl pressed (but with Ctrl-drag, actual result of adding contact1-henry only may have been correct - see comment 12).
  • Same behaviour as in TB 91, current AB.

Thanks Günter for reporting this!

Also confirming for Daily TB 98.0a1 (2022-01-19) (64-bit), where the symptoms are even worse:

  • drag image correctly shows contact 2 (jane) all the way (see video in my next comment)
  • then surprisingly drops contact 1 (henry) instead.

In terms of UX, this constitutes a significant and unexpected violation of privacy: Users might inadvertently send stuff to the wrong address, because the interface is in a different state than they expected it to be (UX-mode-error).

Severity: -- → S2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Summary: DragNDrop from new Addressbook uses wrong address → Dragging unselected contact B from the new Address Book drops selected contact A instead (mail.addr_book.useNewAddressBook=true)

Daily perfects the deception of this bug by showing the right contact during drag and then dropping another.

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

Also confirming for Daily TB 98.0a1 (2022-01-19) (64-bit), where the symptoms are even worse:

  • drag image correctly shows contact 2 (jane) all the way (see video in my next comment)
  • then surprisingly drops contact 1 (henry) instead.

In terms of UX, this constitutes a significant and unexpected violation of privacy: Users might inadvertently send stuff to the wrong address, because the interface is in a different state than they expected it to be (UX-mode-error).

STR in my comment 7. This should be fixed asap by implementing the expected behaviour of comment 7 (same as TB 91 current AB and other apps).

Whiteboard: [STR in comment 7]

Comment on attachment 9259803 [details]
Screencast 1: Expected: TB 91.5.0 old AB, DnDContactsFromABtoComposition.mp4

Screencast 1 (attachment 9259803 [details]) shows the expected behaviour on TB 91.5.0 old AB.

Attachment #9259803 - Attachment description: Screencast 1: DnDContactsFromABtoComposition.mp4 → Screencast 1: Expected: TB 91.5.0 old AB, DnDContactsFromABtoComposition.mp4

Comment on attachment 9259807 [details]
Screencast 2: The edge case: TB 91.5.0 old AB, Ctrl+DnD_ContactsFromABtoComposition.mp4

Screencast 2 (Attachment 9259807 [details]) shows the current behavior on TB 91 old AB for Ctrl+drag of unselected contact with another selected contact (drag only the selected contact). I thought I had the rationale of this edge case behaviour covered in my comment 5, but it turns out Windows 10 Explorer behaviour (which may have some de facto normative value) is different. Ctrl-drag will select the second item too before dragging both items with Copy flavor. So Ctrl actually has two functions here, and Windows Explorer went for applying them both at the same time. Which may or may not be intended.

Attachment #9259807 - Attachment description: Screencast 2: Ctrl+DnD_ContactsFromABtoComposition.mp4 → Screencast 2: The edge case: TB 91.5.0 old AB, Ctrl+DnD_ContactsFromABtoComposition.mp4

Comment on attachment 9259818 [details]
Screencast 3: The bug on TB91.5.0: NewAB_DnDFail.mp4

Screencast 3 (Attachment 9259818 [details]) shows the failure of this bug on TB 91 with new AB enabled.

Attachment #9259818 - Attachment description: Screencast 3: TB91_NewAB_DnDFail.mp4 → Screencast 3: The bug on TB91.5.0: NewAB_DnDFail.mp4

Comment on attachment 9259836 [details]
Screencast 4: The bug on Daily TB 98.0a1 (2022-01-19): New AB DnD fail.mp4

Screencast 4 (Attachment 9259836 [details]) shows the bug on Daily, where it's even more deceptive.

Attachment #9259836 - Attachment description: Screencast 4: TB 98.0a1 (2022-01-19) New AB DnD fail.mp4 → Screencast 4: The bug on Daily TB 98.0a1 (2022-01-19): New AB DnD fail.mp4

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

STR in my comment 7. This should be fixed asap by implementing the expected behaviour of comment 7 (same as TB 91 current AB and other apps).

Is this easy to fix? Seems pretty important

Flags: needinfo?(geoff)

It's easy to fix at the simplest level, which I will do here as this is pretty annoying, but it's not so easy to fix it well and have the selection updated as you'd probably expect. I've been waiting on Henry's other work on this list widget in the hope it would make things better, but so far it hasn't.

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(geoff)

This simply replaces the cards to be dragged with the one under the pointer, if it's not part of the selection. It's not perfect but it's better than the current situation.
Also fixes a bunch of test code that wasn't quite testing the right thing, due to the bad behaviour.

Target Milestone: --- → 104 Branch
Duplicate of this bug: 1778025

(In reply to Geoff Lankow (:darktrojan) from comment #17)

It's easy to fix at the simplest level, which I will do here as this is pretty annoying, but it's not so easy to fix it well and have the selection updated as you'd probably expect. I've been waiting on Henry's other work on this list widget in the hope it would make things better, but so far it hasn't.

As further explanation, from bug 1778025 comment 1

the selection happens on "click" rather than "mousedown". I tried to change it to "mousedown" but a change in selection rebuilds the DOM nodes, so destroys the original Event target, which means the drag event is prevented. See https://phabricator.services.mozilla.com/D149421#4892618

Selecting on "mousedown" won't be easy with the current implementation because it uses nsITreeSelection. But it should be easy after bug 1752532, but that has had to go on the back-burner for 102.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/f0431f51e754
Drag the right address book card if it isn't selected. r=henry

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED

Comment on attachment 9284893 [details]
Bug 1732553 - Drag the right address book card if it isn't selected. r=henry

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Dragging contacts is a pain
Testing completed (on c-c, etc.): 3 days
Risk to taking this patch (and alternatives if risky): Low

Attachment #9284893 - Flags: approval-comm-beta?

Comment on attachment 9284893 [details]
Bug 1732553 - Drag the right address book card if it isn't selected. r=henry

[Triage Comment]
Approved for beta

Attachment #9284893 - Flags: approval-comm-beta? → approval-comm-beta+

Comment on attachment 9284893 [details]
Bug 1732553 - Drag the right address book card if it isn't selected. r=henry

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Dragging contacts is a pain
Testing completed (on c-c, etc.): In 103 b6
Risk to taking this patch (and alternatives if risky): Low

Attachment #9284893 - Flags: approval-comm-esr102?

Comment on attachment 9284893 [details]
Bug 1732553 - Drag the right address book card if it isn't selected. r=henry

[Triage Comment]
Approved for esr102

Attachment #9284893 - Flags: approval-comm-esr102? → approval-comm-esr102+
You need to log in before you can comment on or make changes to this bug.