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

VERIFIED FIXED in Firefox 3.6a1

Status

()

Firefox
Address Bar
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: faaborg, Assigned: Mardak)

Tracking

({verified1.9.1})

Trunk
Firefox 3.6a1
verified1.9.1
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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).
(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.
(Assignee)

Comment 2

9 years ago
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.
(Reporter)

Comment 3

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

sounds good, adjusting summary
Summary: When the user selects to only display history in the location bar, should we display bookmarks that are in history → When the user selects to only display history in the location bar, don't display stars for items that happen to be bookmarked
(Assignee)

Comment 4

9 years ago
Do we want to show tags?
Hardware: x86 → All
(In reply to comment #4)
> Do we want to show tags?

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

Comment 6

9 years ago
Right, but the concern is that it's obvious the entry is bookmarked because there are tags next to it.
(Reporter)

Comment 7

9 years ago
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.
Summary: When the user selects to only display history in the location bar, don't display stars for items that happen to be bookmarked → 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
(Assignee)

Comment 8

9 years ago
Created attachment 369460 [details] [diff] [review]
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..
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #369460 - Flags: review?(gavin.sharp)
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...
Attachment #369460 - Flags: review?(gavin.sharp) → review-
(Assignee)

Comment 10

9 years ago
Created attachment 369674 [details] [diff] [review]
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.
Attachment #369460 - Attachment is obsolete: true
Attachment #369674 - Flags: review?(gavin.sharp)
Hmm, actually, can't this be fixed entirely from within nsNavHistory::AutoCompleteProcessSearch, by checking for mAutoCompleteCurrentBehavior==kAutoCompleteBehaviorHistory when setting the autocomplete entry's style?
(Assignee)

Comment 12

9 years ago
Created attachment 371191 [details] [diff] [review]
v3

Switch to NavHistoryAutoComplete implementation with test.
Attachment #369674 - Attachment is obsolete: true
Attachment #371191 - Flags: review?(dietrich)
Attachment #369674 - Flags: review?(gavin.sharp)
That will strip tags if mAutoCompleteCurrentBehavior is kAutoCompleteBehaviorHistory & kAutoCompleteBehaviorTag, won't it? I suggested == for a reason :)
(Assignee)

Comment 14

9 years ago
Well, it won't strip /because/ it's "& Tag", but sure, if it's "& History" and "& Tag", it'll strip tags.
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.
(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.
(Assignee)

Comment 17

9 years ago
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..
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
(Assignee)

Comment 19

9 years ago
Created attachment 375196 [details] [diff] [review]
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 ^.
Attachment #371191 - Attachment is obsolete: true
Attachment #375196 - Flags: review?(dietrich)
Attachment #371191 - Flags: review?(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.
Attachment #375196 - Flags: review?(dietrich) → review+
(Assignee)

Comment 21

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

Comment 22

9 years ago
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 ^.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
(Assignee)

Comment 23

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

(In reply to comment #20)
> this is ok for 3.5, r=me.
Attachment #375196 - Flags: approval1.9.1?
Attachment #375196 - Flags: approval1.9.1? → approval1.9.1+
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.
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
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1

Updated

9 years ago
Depends on: 494224
You need to log in before you can comment on or make changes to this bug.