360858 - Escape should cancel active edit in Bookmark Manager

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[JK]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061109 Camino/1.1a1+ (jedik)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1) Gecko/20061109 Camino/1.1a1+

In the Bookmarks page, ESC currently does not revert an in-progress edit and cancels data editing. Instead, ESC should cancel the in-progress
(uncommitted) edit.

Reproducible: Always

Steps to Reproduce:
1. CMD-B
2. Start editing something
4. Hit ESC
3. CMD-W
Actual Results:  
Changes saved

Expected Results:  
Reverts edition (back to original data)

Maybe its fix is similar to https://bugzilla.mozilla.org/show_bug.cgi?id=325845

Comment 1

[froodian (Ian Leue)]

Confirming.  Esc currently does nothing, so tweaking summary, but it should cancel.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Mass un-setting milestone per 1.6 roadmap.

Filter on RemoveRedonkulousBuglist to remove bugspam.

Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

Comment 3

[Chris Lawson (gone)]

The fix, as jedik noted, is indeed similar, although the proof-of-concept patch I'm about to post has a bit of a flaw :-\

Comment 4

[Chris Lawson (gone)]

Attachment: POC patch

The flaw is noted in the comment...when you hit Escape with this patch, focus runs off and hides somewhere, which isn't ideal behaviour. In the Location bar, for instance, we revert the changes but leave focus (and selection) in that text field.

Finder cancels the "edit" mode and reverts the text if you hit Escape while editing a filename, and since most of our bookmark manager behaviour is modeled after the Finder, I think that's what should happen here.

I'm just not entirely sure how to make it happen.

Comment 5

[Chris Lawson (gone)]

murph, you've done a lot with the responder chain and focus stuff -- can you help out with comment 4 here?

Comment 6

[Chris Lawson (gone)]

Attachment: fix v1.0

I had an idea tonight that maybe I could store the firstResponder and then manually set it again after aborting the edit, and lo and behold, that works.

Comment 7

[Christopher Henderson]

Comment on attachment 352282 [details] [diff] [review]
fix v1.0

Works as advertised, and most of the code is already used in similar situations elsewhere.

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 352282 [details] [diff] [review]
fix v1.0

sr=pink

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Checked in on cvs trunk.