Closed Bug 440403 Opened 16 years ago Closed 16 years ago

Esc commits edits when closing preferences

Categories

(Camino Graveyard :: Preferences, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla-graveyard, Assigned: bugzilla-graveyard)

Details

(Whiteboard: p-safari)

Attachments

(1 file, 1 obsolete file)

This is sort of a thorny problem, but Smokey and I both agreed on IRC last night that the behaviour is unexpected, so I'm filing it as a bug.

When editing any of our text fields in Preferences, hitting Escape closes prefs and also commits the edit. This means the only practical way to undo what you just did is the Undo command.

In Safari, Escape closes prefs but cancels the active edit rather than committing it, similar to what we dealt with (though on a much smaller scale) in bug 325845. Closing and *not* committing makes a lot more sense than closing and committing, IMO.

I think the "right approach" for this is probably something similar to what was done over there, but it's going to involve adding an |updateUI| method to all our preference panes. Because this would be a mildly large change, I wanted to get some comments on the approach first and see if maybe there's a better way to fix this.
Whiteboard: p-safari
Assignee: nobody → cl-bugs-new
Status: NEW → ASSIGNED
You could probably just make cancel: call |currentEditor] abortEditing]| for each text field.
Attached patch fix v1.0 (obsolete) — Splinter Review
Implements comment 1.
Attachment #339810 - Flags: review?(stuart.morgan+bugzilla)
Comment on attachment 339810 [details] [diff] [review]
fix v1.0

>+  if ( [[[self window] firstResponder] isKindOfClass:[NSTextView class]] && [[self window] fieldEditor:NO forObject:nil] != nil ) {

Lose the spacing inside the (), and break this after &&. r=me with those changes.
Attachment #339810 - Flags: review?(stuart.morgan+bugzilla) → review+
Attached patch fix v1.01Splinter Review
Addresses review comments, how about pink for sr?
Attachment #339810 - Attachment is obsolete: true
Attachment #340822 - Flags: superreview?(mikepinkerton)
Attachment #340822 - Flags: superreview?(mikepinkerton) → superreview+
Comment on attachment 340822 [details] [diff] [review]
fix v1.01

sr=pink. is checking NSTextView sufficient? what about NSTextField?
[12:07pm] cl: pinkerton: to answer your question, I basically stole that code from an example Apple used in the docs
[12:08pm] pinkerton: doesn't mean they thought it through fully :)
[12:08pm] smorgan: pinkerton: NSTextField isn't an editor
[12:09pm] cl: I was just consulting the docs to see about that. Thanks, Stuart.
[12:09pm] pinkerton: ok
[12:09pm] smorgan: pinkerton: text fields do the crazy thing where they swap in the window's shared NSTextView when you start editing
[12:10pm] cl: someone should comment for posterity in the bug so we don't forget.
Keywords: checkin-needed
Whiteboard: p-safari → p-safari [land after freeze]
Checked in on cvs trunk.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: p-safari [land after freeze] → p-safari
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: