286041 - Allow people to undo "forget search" (FORGET SEARCH in list screen too close to EDIT SEARCH)

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement)

Description

[Steve Weisberg]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
Build Identifier: 

On the bottom of the list screen when running a saved search, the "Forget 
Search search-name" is rather close to the "Edit Search".  This makes it much 
too easy to unintentially hit.

Reproducible: Always

Steps to Reproduce:
1.Choose any search
2.Produce the list
3.See list of options on the bottom.

Actual Results:  
One of our users unintentially hit forget search.

Expected Results:  
Any of space it better, different line, different order.

Comment 1

[Stephen Lee]

Perhaps the problem isn't so much the location of the "Forget" link, but the 
fact that "Forget" is a single-click operation.

Might be better to have a column of checkboxes, and a [Forget marked searches] 
button at the bottom. c.f. deleting emails from a list in webmail app.

Comment 2

[Bill Barry]

Should the forget search link even be there? Users can go to their prefs and delete their saved searches from there.

Comment 3

[Frédéric Buclin]

Yes it should still be there. But probably a JS popup asking you if that's really what you want to do would be welcome. This is very irritating deleting a saved search when it took several minutes to get it working correctly (depending on the complexity of your query).

Comment 5

[Kent Rogers [:mrbball]]

It seems that this could be resolved with a very simple "confirm-forget"
template (very similar to the admin/keywords/confirm-delete.html.tmpl
template) and one extra remaction (e.g. "forget-confirmed") in buglist.cgi.

It took me just a few minutes to throw something together for my local
installation and if this is an acceptable approach, I will clean it up
and submit for review.

Comment 6

[Steve Weisberg]

That's great. In fact, if I were opening this bug today I would probably have requested confirmation and not (just) moving the text.

Comment 7

[Max Kanat-Alexander]

Generally it's better UX to offer an undo than a confirmation, if possible,

Comment 8

[Kent Rogers [:mrbball]]

Are there any other examples in Bugzilla where an undo is offered?  All
of the examples I could think of where an entity is being deleted (e.g.
keywords) do a confirmation.

Comment 9

[Max Kanat-Alexander]

I don't think it's fully relevant what Bugzilla does elsewhere--often "what Bugzilla does" is a bad example to go on, unfortunately.

It's fully possible to *offer* an undo in this case (since we know all the data about the thing that was just deleted), so I think that we should.

Comment 10

[Kent Rogers [:mrbball]]

(In reply to comment #7)
> Generally it's better UX to offer an undo than a confirmation, if possible,

I would tend to disagree, especially in the case where the action is 
relatively rare (like forgetting a saved search).

(In reply to comment #9)
> It's fully possible to *offer* an undo in this case (since we know all the
> data about the thing that was just deleted), so I think that we should.

We don't quite know all the data about the saved search being deleted.
We know the name, but not the query itself.  It wouldn't be difficult
to obtain, but I still prefer a confirmation.

Either way, I have patches ready to go.  Heck, we could offer a
confirmation AND an undo.

Comment 11

[Max Kanat-Alexander]

  We can know the query itself; it wouldn't be too hard, particularly when we're using Search::Saved objects.

  This is where my viewpoint on undo vs. confirm came from:

  http://www.alistapart.com/articles/neveruseawarning/

Comment 12

[Kent Rogers [:mrbball]]

Attachment: v1 - undo

>   This is where my viewpoint on undo vs. confirm came from:
> 
>   http://www.alistapart.com/articles/neveruseawarning/

Never say never!  I would agree that there are times when an undo is
preferable, but I would definitely pick the confirmation in this case.

Regardless, here is the patch to offer an undo.  Let me know what you
think.

Comment 13

[Max Kanat-Alexander]

Comment on attachment 429278 [details] [diff] [review]
v1 - undo

Looks awesome! Except that it seems to have a slight bug. When I went to un-forget, I got something like this:

buglist.cgi?newquery=bug_status=UNCONFIRMED&bug_status=...

Maybe you need to urlquote the buffer?

Comment 14

[Kent Rogers [:mrbball]]

Attachment: v2

(In reply to comment #13)
> Maybe you need to urlquote the buffer?

Thank you.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 429311 [details] [diff] [review]
v2

$qname should be url-quoted too. Try ";bug_status=BLAH" or "%3D" as a search name.

Comment 16

[Kent Rogers [:mrbball]]

Attachment: v3

(In reply to comment #15)
> $qname should be url-quoted too.

I remember thinking about that; not sure why I didn't do it ...

Comment 17

[Max Kanat-Alexander]

Comment on attachment 429515 [details] [diff] [review]
v3

This looks great. :-) Thanks for this simple and helpful patch. :-)

Comment 18

[Max Kanat-Alexander]

I'm going to consider this an enhancement, for 3.8. If anybody has any arguments to the contrary, let me know.

Comment 19

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified buglist.cgi
modified template/en/default/global/messages.html.tmpl
Committed revision 7039.

Comment 20

[Steve Weisberg]

Shouldn't the summary-subject of this bug be modified to reflect the current solution? Or am I being too pedantic? :)

Comment 21

[Steve Weisberg]

(In reply to comment #20)
> Shouldn't the summary-subject of this bug be modified to reflect the current
> solution? Or am I being too pedantic? :)

whoops, already done. sorry.

Comment 22

[Max Kanat-Alexander]

Added to the release notes in bug 604256.