User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060728 Camino/1.0+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060728 Camino/1.0+ Changes to the location bar in Camino are added to the undo chain. However, navigating to another page is not. Therefore, pressing undo after navigating to another page (or if the contents of the location bar is transformed in any way) causes strange results. Reproducible: Always Steps to Reproduce: 1. Open a browser window. 2. Enter "mozilla.org" into the location bar and press enter. 2. Enter "caminobrowser.org" into the location bar and press enter. The location bar will change to "http://caminobrowser.org/" 3. Click in the location bar and press undo. Actual Results: The address changes to "http://www.mozilla.org/browser.org/". Expected Results: Camino should either not allow me to undo after navigating or make a sane decision as to what the location bar should be after undo.

Undo behaviour in this case is also going to have to take into account whatever sort of session saving/tab undo we implement. If a keypress results in browser activity (rather than simply an edit to the location bar), Undo should function like Back, or simply not do anything (if functioning like Back doesn't make sense). If the keypress does not result in browser activity, Undo should undo the typing. cl

BTW, Safari's behaviour is what's proposed here -- if typing results in navigation, Undo does nothing. If typing does not result in navigation, Undo un-does the typing. Confirming and targeting for 1.2. I might take a look at this soon-ish.

In other words, navigating clears the location bar's Undo buffer?

(In reply to comment #3) > In other words, navigating clears the location bar's Undo buffer? Yeah, that's what's being proposed. cl

Gross. I noticed this while testing a fix for bug 409634.

-[AutoCompleteTextField setURI:] isn't telling the undo stack about the changes it's making in the "not currently editing" case. The state of the text field is out of sync with what the undo stack thinks, so undo operations don't have the expected effect.

Actually, our code already tries to purge the undo stack of actions that affect the location bar once editing is done, but it's ineffective because of a bug elsewhere. Here's where we try to take the text field edits off of the undo stack: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/camino/src/browser/AutoCompleteTextField.mm&rev=1.56&mark=1139-1140#1136 The field editor for the location bar is an AutoCompleteTextFieldEditor (NSTextView subclass), defined in BrowserWindowController.mm. It maintains its own NSUndoManager instance that BrowserWindowController makes the window's active undo manager while the text field is first responder. The bug is that AutoCompleteTextFieldEditor makes its own NSUndoManager available through a selector named undoManagerForTextView:, which is actually an NSTextView *delegate* method - an NSTextView should *call* this, not *implement* it. An NSView (super-superclass of NSTextView) is supposed to make its NSUndoManager available at the undoManager selector. Thanks, David Haas. What happened was that the code above would look for its undo manager using the standard mechanism but wind up getting the window's undo manager, which was wrong, because that wasn't the undo manager that got the text editing events pushed ont it and it wasn't the undo manager that was hooked up to the edit menu and command-Z while editing. There would be nothing relevant to remove from the window's undo manager, and the next time the field was edited, it would bring along its own undo manager again, prepopulated with changes from the previous edit, which didn't make any sense given that the field's contents likely changed since the last time it was edited due to things like navigation and adding "http://". The fact that you could undo across editing sessions in the location bar was an unintended misfeature, and a buggy one at that. Let's kill it!

Comment on attachment 294461 [details] [diff] [review] Implement the correct selector, don't allow undo across location bar editing sessions code r=me, but I can't test at the moment.

Checked in on the trunk and MOZILLA_1_8_BRANCH for 1.6b1: trunk/mozilla/camino/src/browser/AutoCompleteTextField.mm 1.59 trunk/mozilla/camino/src/browser/BrowserWindowController.mm 1.349 1.8/mozilla/camino/src/browser/AutoCompleteTextField.mm 1.37.2.22 1.8/mozilla/camino/src/browser/BrowserWindowController.mm 1.201.2.144