User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 After clearing all private data (Ctrl-Shift-Del, checking all boxes), you can select either the URL address bar or the built in search bar and hit Ctrl-Z. This will cycle through all of the previously visited addresses even though I have already cleared all history. This works in both the URL and search fields. Reproducible: Always Steps to Reproduce: 1. Visit a few web sites and/or make a few searches from the built in search bar. 2. Clear all private data (Ctrl-Shift-Del) 3. Select either the URL address bar or the search bar. 4. Hit Ctrl-Z to repeatedly cycle through previously viewed URLs and/or search requests. Actual Results: I can see all previously viewed URLs. Expected Results: I see nothing since I've already cleared all private data. I don't know if this is a Firefox 1.5 problem or an OS problem. I'm running WinXP SP2 with all updates.

I guess I'm responsible for this.

There's a known problem where "clear private data" doesn't clear session history (back-forward buttons) for which the workaround is to open a new tab (or window) and close the old one. This has a similar workaround: close the current window to clear the edit history stored in the address widget. Sanitize Browsing History should zap the location bar undo history in all open browser windows. Sanitize Form Data should do the same for the undo history of the search box.

I don't think fixing bug 158071 is responsible. We want that bug to remain fixed, we want Sanitize to take proactive steps to tell these edit boxes to forget their undo history.

so I guess the patch has to be something like this.

Does the patch fix the search bar and the URL bar, or just the URL bar?

(In reply to comment #5) > Does the patch fix the search bar and the URL bar, or just the URL bar? It depends on which boxes are checked when clearing data. If "Browsing History" is checked, then the URL bar history should be cleared. If "Saved Form Information" is checked, then the search bar history should be cleared.

No, it just fixes the URL Bar, I concentrated on what I regressed. I'll see what I can do for the search bar.

Ok, this patch is actually good. It also takes care of clearing the undo history of the search bar (which was actually something I regressed after all). If bug 312867 would be fixed in Firefox, then I would not need to QueryInterface for the editor with the searchbar (for the urlbar, it's somehow working already).

Comment on attachment 209443 [details] [diff] [review] patch Apostrophitis: its undo history.

(In reply to comment #8) >(for the urlbar, it's somehow working already). I guess as a result of the newline stripping patch.

(In reply to comment #8) > If bug 312867 would be fixed in Firefox I just landed the patch for that bug (exposing "editor").

Ok, removed the QI and fixed my apostrophitis.

Comment on attachment 209579 [details] [diff] [review] patch2 >Index: browser/base/content/search.xml > doCommand: function (aCommand) > { >+ // Clear the searchbar and its undo history >+ this.mOuter.value = ""; >+ this.mOuter.mInputField.editor.enableUndo(false); >+ this.mOuter.mInputField.editor.enableUndo(true); You can just use "this.mOuter.editor", given the patch in the other bug, no? Same comment for the other ones (gURLBar.editor and searchBar.mTextbox.editor). >Index: browser/base/content/browser.js >+ //Clear undo history of all searchBars URL Bars?

Oops, you're right. This patch fixes that.

Comment on attachment 209704 [details] [diff] [review] patch3 >Index: browser/base/content/browser.js >+ //Clear undo history of all searchBars URL bars? :) >Index: browser/base/content/sanitize.js >+ if (searchBar) { >+ searchBar.mTextbox.mInputElt.value = ""; Just mTextbox.value? >+ const nsIDOMNSEditableElement = Components.interfaces.nsIDOMNSEditableElement; No need for this, right?

Thanks, I fixed those issues.

Comment on attachment 209705 [details] [diff] [review] patch >+ //Clear undo history of all URL Bars >+ var windowManager = Components.classes['@mozilla.org/appshell/window-mediator;1'].getService(); >+ var windowManagerInterface = windowManager.QueryInterface(Components.interfaces.nsIWindowMediator); >+ var windows = windowManagerInterface.getEnumerator("navigator:browser"); >+ while (windows.hasMoreElements()) { >+ var urlBar = windows.getNext().gURLBar; >+ if (urlBar) { >+ gURLBar.editor.enableUndo(false); >+ gURLBar.editor.enableUndo(true); >+ } >+ } I'm sure you meant urlbar here! r=me with that fixed

Oops, yes, that was an obvious mistake.

mozilla/browser/base/content/browser.js; new revision: 1.559; mozilla/browser/base/content/sanitize.js; new revision: 1.14; mozilla/browser/base/content/search.xml; new revision: 1.36;

Comment on attachment 210265 [details] [diff] [review] patch5 >Index: browser/base/content/search.xml >=================================================================== >RCS file: /cvsroot/mozilla/browser/base/content/search.xml,v >retrieving revision 1.35 >diff -u -8 -p -r1.35 search.xml >--- browser/base/content/search.xml 11 Jan 2006 20:24:20 -0000 1.35 >+++ browser/base/content/search.xml 26 Jan 2006 15:52:02 -0000 >@@ -384,19 +384,23 @@ > isCommandEnabled: function (aCommand) > { > return this.mOuter.formHistSvc.nameExists( > this.mOuter.getAttribute("autocompletesearchparam")); > }, > > doCommand: function (aCommand) > { >+ // Clear the searchbar and its undo history >+ this.mOuter.value = ""; >+ this.mOuter.editor.enableUndo(false); >+ this.mOuter.editor.enableUndo(true); >+ > this.mOuter.formHistSvc.removeEntriesForName( > this.mOuter.getAttribute("autocompletesearchparam")); >- this.mOuter.value = ""; > } > }) > ]]></field> > > <!-- DND Observer --> > <field name="searchbarDNDObserver" readonly="true"><![CDATA[ > ({ > mOuter: this, >Index: browser/base/content/browser.js >=================================================================== >RCS file: /cvsroot/mozilla/browser/base/content/browser.js,v >retrieving revision 1.555 >diff -u -8 -p -r1.555 browser.js >--- browser/base/content/browser.js 20 Jan 2006 23:04:16 -0000 1.555 >+++ browser/base/content/browser.js 26 Jan 2006 15:52:11 -0000 >@@ -237,16 +237,28 @@ const gSessionHistoryObserver = { > { > if (topic != "browser:purge-session-history") > return; > > var backCommand = document.getElementById("Browser:Back"); > backCommand.setAttribute("disabled", "true"); > var fwdCommand = document.getElementById("Browser:Forward"); > fwdCommand.setAttribute("disabled", "true"); >+ >+ //Clear undo history of all URL Bars >+ var windowManager = Components.classes['@mozilla.org/appshell/window-mediator;1'].getService(); >+ var windowManagerInterface = windowManager.QueryInterface(Components.interfaces.nsIWindowMediator); >+ var windows = windowManagerInterface.getEnumerator("navigator:browser"); >+ while (windows.hasMoreElements()) { >+ var urlBar = windows.getNext().gURLBar; >+ if (urlBar) { >+ urlBar.editor.enableUndo(false); >+ urlBar.editor.enableUndo(true); >+ } >+ } > } > }; > > /** > * Given a starting docshell and a URI to look up, find the docshell the URI > * is loaded in. > * @param aDocument > * A document to find instead of using just a URI - this is more specific. >Index: browser/base/content/sanitize.js >=================================================================== >RCS file: /cvsroot/mozilla/browser/base/content/sanitize.js,v >retrieving revision 1.11 >diff -u -8 -p -r1.11 sanitize.js >--- browser/base/content/sanitize.js 8 Oct 2005 15:44:47 -0000 1.11 >+++ browser/base/content/sanitize.js 26 Jan 2006 15:52:12 -0000 >@@ -187,16 +187,29 @@ Sanitizer.prototype = { > .getService(Components.interfaces.nsIBrowserHistory); > return globalHistory.count != 0; > } > }, > > formdata: { > clear: function () > { >+ //Clear undo history of all searchBars >+ var windowManager = Components.classes['@mozilla.org/appshell/window-mediator;1'].getService(); >+ var windowManagerInterface = windowManager.QueryInterface(Components.interfaces.nsIWindowMediator); >+ var windows = windowManagerInterface.getEnumerator("navigator:browser"); >+ while (windows.hasMoreElements()) { >+ var searchBar = windows.getNext().document.getElementById("searchbar"); >+ if (searchBar) { >+ searchBar.mTextbox.value = ""; >+ searchBar.mTextbox.editor.enableUndo(false); >+ searchBar.mTextbox.editor.enableUndo(true); >+ } >+ } >+ > var formHistory = Components.classes["@mozilla.org/satchel/form-history;1"] > .getService(Components.interfaces.nsIFormHistory); > formHistory.removeAllEntries(); > }, > > get canClear() > { > var formHistory = Components.classes["@mozilla.org/satchel/form-history;1"]

Checked in on the 1.8 branch. mozilla/browser/base/content/browser.js; new revision: 1.479.2.54; mozilla/browser/base/content/sanitize.js; new revision: 1.5.2.9; mozilla/browser/base/content/search.xml; new revision: 1.25.2.8;

Comment on attachment 210265 [details] [diff] [review] patch5 >+ this.mOuter.editor.enableUndo(false); >+ this.mOuter.editor.enableUndo(true); >+ urlBar.editor.enableUndo(false); >+ urlBar.editor.enableUndo(true); >+ searchBar.mTextbox.editor.enableUndo(false); >+ searchBar.mTextbox.editor.enableUndo(true); ^^^ What happens when enableUndo() has already been set to false? Wouldn't this set it to true (maybe causing something else to mess up)? I'm not sure if this would ever happen or cause a real problem, but I thought I should bring it up.

I am unaware of any situation where we support, or care about, a case where someone has disabled undo in the URL bar or search textboxes.

Well, maybe a special xul attribute (disableundo="true" or something) could be created, so it would be easy to keep track of this. But I doubt if it would used much.

Doesn't look like this was really wanted in 1.8.0, and it's too late now for .0.2 for a bug that isn't fixing a critical security hole. If you still want this for the 1.8.0 branch please nominate for 1.8.0.3 and request approval on the patch.