485122 - When the user selects to only display history in the location bar, don't display the star and tag icons for items that happen to be bookmarked or tagged

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

We need to make a decision on this, right now it feels a little odd to see bookmarks in the location bar after explicitly asking for them not to be displayed.  I think the assumption is probably that the user is combining this preference with actively clearly their history (either on close, or by starting in private browsing mode), so they will only see these items after visiting them in the current session.

One possibility would be to not display a bookmark star for items in the location bar autocomplete results that are only being displayed because they are in history (even if they are in fact bookmarks).

Comment 1

[(no longer active)]

(In reply to comment #0)
> One possibility would be to not display a bookmark star for items in the
> location bar autocomplete results that are only being displayed because they
> are in history (even if they are in fact bookmarks).

I think this is the most natural way to handle this.  Excluding history items because they are bookmarked as well doesn't seem logical to me.

Comment 2

[Ed Lee :Mardak]

I agree. We obey the user pref and only show history items.. some just happen to be bookmarked.

We can just check the pref and not show a star.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

>We can just check the pref and not show a star.

sounds good, adjusting summary

Comment 4

[Ed Lee :Mardak]

Do we want to show tags?

Comment 5

[(no longer active)]

(In reply to comment #4)
> Do we want to show tags?

Yes, I think so, because the pref doesn't concern tags.

Comment 6

[Ed Lee :Mardak]

Right, but the concern is that it's obvious the entry is bookmarked because there are tags next to it.

Comment 7

[Alex Faaborg [:faaborg] (Firefox UX)]

yeah, and we are trying to establish that when you tag something it is also bookmarked (you can't tag history items), so it probably makes sense to hide the tags as well.

Comment 8

[Ed Lee :Mardak]

Attachment: v1

I wonder if we should have autocomplete binding call some dummy adjust item method set by urlbarbindings to process the item before being displayed..

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 369460 [details] [diff] [review]
v1

We shouldn't getService the preferences service for each adjustItem call, and ideally shouldn't get the pref more than once per _invalidate. That pref is also specific to places, and the toolkit autocomplete binding isn't... my patch for bug 480350 adds an "insertion watcher" which could be useful here, I guess.

What's type="favicon"? Don't see that being used anywhere at the moment...

Comment 10

[Ed Lee :Mardak]

Attachment: v2

> What's type="favicon"? Don't see that being used anywhere at the moment...
It's the default type that nsNavHistoryAutoComplete uses if it isn't anything else.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Hmm, actually, can't this be fixed entirely from within nsNavHistory::AutoCompleteProcessSearch, by checking for mAutoCompleteCurrentBehavior==kAutoCompleteBehaviorHistory when setting the autocomplete entry's style?

Comment 12

[Ed Lee :Mardak]

Attachment: v3

Switch to NavHistoryAutoComplete implementation with test.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

That will strip tags if mAutoCompleteCurrentBehavior is kAutoCompleteBehaviorHistory & kAutoCompleteBehaviorTag, won't it? I suggested == for a reason :)

Comment 14

[Ed Lee :Mardak]

Well, it won't strip /because/ it's "& Tag", but sure, if it's "& History" and "& Tag", it'll strip tags.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Edward points out that I forgot that the bitfield represents a set of restrictions again...

Still, it seems like a query restricted to tagged history entries shouldn't be stripped of the tag indicator.

Comment 16

[Dietrich Ayala (:dietrich)]

(In reply to comment #15)
> Still, it seems like a query restricted to tagged history entries shouldn't be
> stripped of the tag indicator.

agreed. my concern is that users will think of these search operators as overrides to the default behavior. eg: "if i restrict to history by default, i'll still be able to search bookmarks and tags via the operators".

if we ignore the operators at any point, then the software is just not doing what the user asked it to. which is bad.

Comment 17

[Ed Lee :Mardak]

The software is still filtering the results by bookmarks if you type *. It just won't give away that those pages are your bookmarks.

I think it would be more jarring to the user if somebody could come over to his/her machine and hit * and reveal the bookmarks that s/he thought weren't marked as bookmarks.

But again.. this is assuming obscurity is a good way to keep things safe..

Comment 18

[Dietrich Ayala (:dietrich)]

We talked about this today at the all-hands. Basically: In Firefox 3 we put a bunch of effort into showing users *why* awesomebar results match, via visual indicators such as the star and tag icons. With this change, when the user explicitly *asks* for bookmarks (for example) we won't show the visual indicator that shows that their search worked.

Here are two possible solutions:

- respect the default behavior, and show the icons only when search operators are used

- override the default behavior when search operators are used

Comment 19

[Ed Lee :Mardak]

Attachment: v3.1

Pretend a page isn't bookmarked/tagged when searching for only history unless the user explicitly wants them. Test by updating special searches test to ignore tags when searching for history ^.

Comment 20

[Dietrich Ayala (:dietrich)]

Comment on attachment 375196 [details] [diff] [review]
v3.1

hrm, i'm not happy w/ special-casing this. i think we should move to a model where all special search chars override the default. but that'd require a bigger change than we want to take. this is ok for 3.5, r=me.

Comment 21

[Ed Lee :Mardak]

Special vs default isn't the issue here. You can trigger a history only search with ^ which would hide the tags and star.

Comment 22

[Ed Lee :Mardak]

http://hg.mozilla.org/mozilla-central/rev/033f19d61c13

Pretend a page isn't bookmarked/tagged when searching for only history unless the user explicitly wants them. Test by updating special searches test to ignore tags when searching for history ^.

Comment 23

[Ed Lee :Mardak]

Comment on attachment 375196 [details] [diff] [review]
v3.1

(In reply to comment #20)
> this is ok for 3.5, r=me.

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 375196 [details] [diff] [review]
v3.1

a191=beltzner, though I think this is potentially confusing in the opposite direction. I'm going to assume you guys have thought the UI design through, here.

Comment 25

[Ed Lee :Mardak]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d88b588f7ed8

Comment 26

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090521 Minefield/3.6a1pre ID:20090521030951

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090521 Shiretoko/3.5pre ID:20090521030957