After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 627034 - Tomboy deletes selected note when you right-click *some other one* and click "Delete"
Tomboy deletes selected note when you right-click *some other one* and click ...
Status: RESOLVED FIXED
Product: tomboy
Classification: Applications
Component: General
1.2.x
Other Linux
: Normal critical
: 1.6.0
Assigned To: Tomboy Maintainers
Tomboy Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-08-16 10:38 UTC by Tomasz Chrzczonowicz
Modified: 2011-08-20 20:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Improvement as suggested above (1.00 KB, patch)
2010-11-19 07:13 UTC, Aaron D Borden
accepted-commit_now Details | Review
Name the note being deleted in confirmation dialog (966 bytes, patch)
2010-11-19 15:29 UTC, Aaron D Borden
committed Details | Review

Description Tomasz Chrzczonowicz 2010-08-16 10:38:05 UTC
Reported downstream at https://bugs.launchpad.net/ubuntu/+source/tomboy/+bug/617365

Steps to reproduce:

0. Open the Search All Notes window.
1. Select some note by left-clicking it. Let's call it "Note 1".
2. Right-click some other note. Let's call it "Note 2"
3. Pick "Delete" from the menu.

What happens:

You get the confirmation dialog. When you press ok, "Note 1" is deleted.

What should happen:

"Note 2" should be deleted.

Package: tomboy 1.2.1-0ubuntu1
Architecture: amd64
Comment 1 Benjamin Podszun 2010-08-16 18:10:58 UTC
This is filed as "critical", yet the source says this:

// Return true so that the base handler won't
// run, which causes the selection to change to
// the row that was right-clicked.
args.RetVal = true;
break;

In other words, someone explicitly made sure that the behavior is as it currently is. I guess I could easily revert it. But - should I?

Anyone following this bug that knows _why_ this behavior was chosen in the first place?
Comment 2 Tomasz Chrzczonowicz 2010-08-16 18:18:53 UTC
I filed is as critical, because https://bugzilla.gnome.org/page.cgi?id=fields.html#importance says:

Critical  	Crashes, causes loss of data, or is a severe memory leak

Well, this is unexpected and for most people practically irrecoverable loss of data.
Comment 3 Benjamin Podszun 2010-08-16 18:23:36 UTC
I agree and - maybe you misunderstood me - didn't question the severity.

I'm just confused because of a critical bug on the one hand vs. the (back then) desired behavior according to the code/comment.

Since I'm new(ish) to Tomboy development this report above was mostly to attract The Old Ones to voice an opinion.
Comment 4 Sandy Armstrong 2010-08-16 18:26:39 UTC
Yeah, this is critical (of course, technically, you don't lose data because the note is moved to ~/.local/share/tomboy/Backup , but still...).

Benjamin, I don't know why that exists in the code.  Maybe it's related to multiple selection right-clicks?
Comment 5 Aaron D Borden 2010-11-19 07:02:48 UTC
commit dc4685a04

It is related to mutliple selection. Removing it causes the item clicked to become selected/deselected. That means multiple selection doesn't work right without it.

I'd say this is by design, because when you right click and hit delete, the item getting deleted is the one highlighted. Visually, we're not doing anything that doesn't make sense.

To improve this:
On right-click,
if we have > 1 items selected, return true (don't change the selection)
else 
  if item right-clicked is not selected then change the selection and open the right-click menu
  else, just open the right-click menu

Although, you could argue this behavior is also a bug.
Comment 6 Aaron D Borden 2010-11-19 07:13:08 UTC
Created attachment 174822 [details] [review]
Improvement as suggested above

Try this out, from a UI perspective, we're changing the behavior when 1 item is selected vs. multiple items selected. Either way, we're only deleteing the notes that are highlighted.
Comment 7 Aaron D Borden 2010-11-19 15:29:32 UTC
Created attachment 174851 [details] [review]
Name the note being deleted in confirmation dialog

Here's another take on this:
The issue here is that the user is not clear on which note is being deleted. Visually, the note that is highlighted is being deleted, but there is an expectation that the note they are right-clicking on should be deleted. By including the name of the note in the confirmation, we remove all ambiguity.

For multiple notes there's no issue, its pretty clear that if you see "Really delete 7 notes?" that somewhere, 7 notes are highlighted and the user should check to see which those are (in case they have been scrolled off-screen).

I prefer this fix to the previous one.
Comment 8 Nicolas C. 2010-12-04 17:03:35 UTC
Actually, the first fix makes a lot of sense. I think it really is what we would intuitively expect as a user. Try it out in Nautilus, it works exactly like that: if you right click on a file that is already selected, whether in a multiple selection or not, it stays selected, if you right-click on a file that is not selected, it gets selected.

The second patch also makes sense, and it would be good to include it, but I would argue against applying only the second one: haven't we all clicked too quickly every once in a while? IMHO, the text being more precise is nice, but it isn't enough. (Arguably, there shouldn't be any confirmation dialog, and a nicer way to cancel the deletion. See this article on A List Apart[1] and bug #382007 about having a trash in Tomboy[2]).

Just my 2c.

[1] http://www.alistapart.com/articles/neveruseawarning/
[2] https://bugzilla.gnome.org/382007
Comment 9 Aaron D Borden 2011-02-20 06:13:29 UTC
Bump: this bug contains a patch with a string change
Comment 10 Sandy Armstrong 2011-08-06 05:40:40 UTC
Review of attachment 174851 [details] [review]:

Commit when ok to commit string changes
Comment 11 Sandy Armstrong 2011-08-06 05:41:23 UTC
Review of attachment 174822 [details] [review]:

Seems good.
Comment 12 Aaron D Borden 2011-08-20 20:54:23 UTC
Committed attachment 174851 [details] [review] as 6a3bb1c863cd775e1eb01780460c27d4457ee96d

This will appear in the 1.7.4 version of tomboy.