If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Escape should cancel active edit in Bookmark Manager

RESOLVED FIXED

Status

Camino Graveyard
Bookmarks
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: JK, Assigned: Chris Lawson (gone))

Tracking

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

1.39 KB, patch
Christopher Henderson
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
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

11 years ago
Confirming.  Esc currently does nothing, so tweaking summary, but it should cancel.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: ESC should cancel incomplete edit, not commit it, when editing bookmark in Bookmarks page → ESC should cancel incomplete edit when editing bookmark in Bookmarks page
Target Milestone: --- → Camino1.2
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*.
Target Milestone: Camino1.6 → ---
Whiteboard: [good first bug]
(Assignee)

Comment 3

9 years ago
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 :-\
Assignee: nobody → cl-bugs-new
Status: NEW → ASSIGNED
Hardware: Macintosh → All
Summary: ESC should cancel incomplete edit when editing bookmark in Bookmarks page → Escape should cancel active edit in Bookmark Manager
(Assignee)

Comment 4

9 years ago
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.
(Assignee)

Comment 5

9 years ago
murph, you've done a lot with the responder chain and focus stuff -- can you help out with comment 4 here?
(Assignee)

Comment 6

9 years ago
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.
Attachment #350106 - Attachment is obsolete: true
Attachment #352282 - Flags: review?(trendyhendy2000)
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.
Attachment #352282 - Flags: review?(trendyhendy2000) → review+
(Assignee)

Updated

9 years ago
Attachment #352282 - Flags: superreview?(mikepinkerton)
Comment on attachment 352282 [details] [diff] [review]
fix v1.0

sr=pink
Attachment #352282 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on cvs trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.