256201 - discoverability of Clear Search History is hard (add a context menu item)

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[chris hofmann]

Jesse comments: doesn't "clear form information" do that anyway?
	
we do get lots of mail to webmaster and other "question/support" aliases on this.. 

tools | options | saved form information  [clear]

to clear form history seems tough for people to assoicate with the search bar...

googlebar for mozilla adds "clear history" on the drop down list, or right click
when hovering over the search bar might do the trick

Comment 1

[Pierre Chanial]

Attachment: patch v1.0

This patch does the following:

- truely makes the searchbar an XBL widget, so that it will be easier to have
several searchbars at the same. (but unless there is a big request for it, I
don't plan to do it for 1.0)
- adds a 'Clear Search History' menuitem to the textbox context menu that
selectively removes the form history entries related with the search bar.
- focus (and eventually selects) the text box when an engine is selected (bug
253015)
- fixes ctrl-down regression (bug 256682)
- clears the text box not just after the user presses return, not after the
search page is loaded, but after the latter is unloaded (bug 253331).
- adds a menuseparator above "Add Engines..." (+capitalizes engine)
- more clean-up after the find in page removal.

I tested all the searchbar functionalities (search, shortcuts, engine
persistence, default engine, dnd in the textbox, pressing enter with no search
string, adding/selecting engines, ctrl-up, ctrl-down navigation, new/old
profile)

Comment 2

[Pierre Chanial]

Comment on attachment 156868 [details] [diff] [review]
patch v1.0

bryner, could have a look to it, and at least review the satchel changes,
please?

Comment 3

[Dorando]

I don't see searchbar.properties in there...

>-#search-bar {
>+/* ::::: search-bar ::::: */
>+
>+search-bar {

searchbar

Comment 4

[Brian Ryner (not reading)]

Comment on attachment 156868 [details] [diff] [review]
patch v1.0

I'm reviewing just the Satchel changes here:

- nameExists seems like a better name than isEmptyForName, because it works
similarly to entryExists.  In fact, it would be great if those could be the
same interface method or at least share an implementation.

- Similarly, I think removeEntriesForName() should share an implementation with
removeAllEntries().

The satchel changes look ok other than that but I'd like to see a revised
patch.

Comment 5

[Pierre Chanial]

Attachment: patch v2.0

patch revised per bryner's and Dorando's comments.
Refactoring has a really good side effect: previous implementation of
EntryExists was returning an error code every time it didn't find a name paired
with a value.

Comment 6

[Pierre Chanial]

Comment on attachment 157463 [details] [diff] [review]
patch v2.0

requesting bryner's review for all the patch.

Comment 7

[Pierre Chanial]

Attachment: patch v2.1

minor class naming tweak

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

There's no way we need a patch this large to implement a feature as small as
clear search history at this stage in the game. Can't we get something simpler? 

Comment 9

[Pierre Chanial]

Attachment: patch v2.2

same patch, without the part to clear the search bar when the result page is
unloaded.

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 157982 [details] [diff] [review]
patch v2.2

I'm OK with the chrome stuff... get bryner to look over the satchel stuff again
and r=me.

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 157982 [details] [diff] [review]
patch v2.2

a=[e-mail]

Comment 12

[Pierre Chanial]

ETA: tuesday morning.

Comment 13

[Pierre Chanial]

fixed on branch

Comment 14

[Mike Kowalski]

I think the patch caused bug 258359.

Comment 15

[Tracy Walker [:tracy]]

confirmed fixed on Windows Firefox Branch 2004-09-10-08-0.9 and Mac Firefox
Branch 2004-09-10-07-0.9

Comment 16

[Uri Bernstein]

This patch seems to have something to do with bug 258943 (crash on OS X when
opening a new window using window.open()). See the first two points of bug
258943 comment 36, and bug 258943 comment 40.

Comment 17

[erich]

Currently, people go to Tools>Options>Privacy to clear the history of everything
BUT the search history.

Having to also go to the search bar specifically to clear the search history
doesn't seem to make a lot of sense.

Why not add "Clear Search History" to the list of options under
Tools>Options>Privacy?

Thanks!

Comment 18

[PikeUK]

*** Bug 240331 has been marked as a duplicate of this bug. ***

Comment 19

[Steffen Wilberg]

Fixed on trunk by the aviary landing.