Closed
Bug 440403
Opened 16 years ago
Closed 16 years ago
Esc commits edits when closing preferences
Categories
(Camino Graveyard :: Preferences, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bugzilla-graveyard, Assigned: bugzilla-graveyard)
Details
(Whiteboard: p-safari)
Attachments
(1 file, 1 obsolete file)
1.12 KB,
patch
|
mikepinkerton
:
superreview+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•16 years ago
|
Whiteboard: p-safari
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → cl-bugs-new
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 1•16 years ago
|
||
You could probably just make cancel: call |currentEditor] abortEditing]| for each text field.
Assignee | ||
Comment 2•16 years ago
|
||
Implements comment 1.
Attachment #339810 -
Flags: review?(stuart.morgan+bugzilla)
Comment 3•16 years ago
|
||
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+
Assignee | ||
Comment 4•16 years ago
|
||
Addresses review comments, how about pink for sr?
Attachment #339810 -
Attachment is obsolete: true
Attachment #340822 -
Flags: superreview?(mikepinkerton)
Updated•16 years ago
|
Attachment #340822 -
Flags: superreview?(mikepinkerton) → superreview+
Comment 5•16 years ago
|
||
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.
Description
•