575609 - "Switch to Tab" autocomplete entries should have icon decorations (star, magnifier glass, et al.) like other entries

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Since the landing of the "Switch to Tab" feature there are no decorations for auto-complete entries in the location bar. That means bookmarks don't have stars, keyword annotated bookmarks don't have a magnifier glass anymore.

As given by limi on bug 573587 comment 7 that shouldn't be the case. Those decorations should be always visible.

Requesting blocking1.9.3 because that's a major ux regression.

Comment 1

[Drew Willcoxon :adw]

The style of switch-to-tab results is set to "action", so all the normal styles ("tag", "bookmark", "favicon", etc.) are lost.  We'll need to set multiple styles.  Taking.

Comment 2

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

Blocks final for polish, but not any specific beta.

Comment 3

[Drew Willcoxon :adw]

Attachment: patch

Treat "action" as an independent toggle switch rather than a mutually exclusive style on par with "tag", "bookmark", et al.

Comment 4

[Elmar Ludwig]

Comment on attachment 455629 [details] [diff] [review]
patch

+          var actionRegexp = /\s*action\s*/;

What is this supposed to do? Match at word boundaries? If so, this will not do what you want, I think.

Comment 5

[Drew Willcoxon :adw]

But the viper still stings, does he not?

Comment 6

[Drew Willcoxon :adw]

Attachment: patch v1.1

OK OK, while the regexp in comment 4 is perfectly fine for this patch, it's not so robust.  It could properly test on word boundaries instead, but word boundaries are zero-length matches, so extra whitespace wouldn't be removed in the subsequent string replacement, and '-', which might appear in some future token, is treated as a word boundary.  So rather than using some gnarly regexp, this patch splits the string on whitespace, looks for "action" in the resulting array, and joins the array sans "action" to reconstruct the string.  That should be slightly more robust in the face of future changes, e.g., if the string needs to be tokenized further or if some future token contains "action" as a substring.

I'll land when I get a chance.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Don't we need an additional review with those changes?

Comment 8

[Drew Willcoxon :adw]

I don't think so, it's pretty trivial.

Comment 9

[Drew Willcoxon :adw]

http://hg.mozilla.org/mozilla-central/rev/f1b1e827d401

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b5pre) Gecko/20100826 Minefield/4.0b5pre