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
Confirming. Esc currently does nothing, so tweaking summary, but it should cancel.
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*.
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 :-\
Created attachment 350106 [details] [diff] [review] 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.
murph, you've done a lot with the responder chain and focus stuff -- can you help out with comment 4 here?
Created attachment 352282 [details] [diff] [review] 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 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 on attachment 352282 [details] [diff] [review] fix v1.0 sr=pink
Checked in on cvs trunk.