Closed
Bug 335441
Opened 20 years ago
Closed 20 years ago
Change tiny drop-down in browser search box into a button/drop-down combo to the right of the search box
Categories
(Firefox :: Search, enhancement)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mozilla, Assigned: pamg.bugs)
References
Details
Attachments
(3 files, 9 obsolete files)
|
1.44 KB,
image/png
|
Details | |
|
16.62 KB,
patch
|
Details | Diff | Splinter Review | |
|
2.80 KB,
image/png
|
Details |
| Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
| Assignee | ||
Updated•20 years ago
|
Assignee: nobody → pamg.bugs
Status: ASSIGNED → NEW
Comment 1•20 years ago
|
||
bug 258672 has a bit of discussion about dropdown and icon placement, fwiw.
| Assignee | ||
Comment 2•20 years ago
|
||
A lot of discussion reached no firm conclusions that I can see. My current working model, for the greatest consistency with the other UI elements (especially the location field), is
.---------------------------.-----+
( [G] Search Google |v ) O. |
`---------------------------'-----+
|[Y] Search Yahoo |
|[a] Search Amazon.com |
|[e] Search eBay |
+---------------------------+
Comment 3•20 years ago
|
||
The drop-down in the location field allows a user to view the previous contents of that field, which is different from selecting a search engine, so in this case I fear the consistency is misplaced (though I'm usually totally a fan! ;)
The function of the button should be to, in order of priority:
1. Perform a search
2. Allow a user to change which engine they are using
I think the button should be a compound like the back/fwd buttons, with the single-click action on the left, and the interim action controlled by the "v" to its right.
So:
.---------------------------.---.
( [G] |O. v) <-- inactive
`---------------------------'---'.
.---------------------------.---.
( [G] foo |O. v) <-- entered search term
`---------------------------'---'.
.---------------------------.---.
( [G] foo |O. v) <-- drop-down clicked
`---------------------------'---'.
| Search with Google [G]|
| Search with Yahoo! [Y]|
|Search with Wikipedia[W]|
'------------------------'
(note: this design assumes no action on bug 258672; any discussion of grey text and logo-in-search field should take place in that bug)
Comment 4•20 years ago
|
||
That's absolutely correct thinking, and visual consistency without functionality consistency is worse than visual inconsistency.
| Assignee | ||
Comment 5•20 years ago
|
||
A good point. (Although the downward-pointing arrow on a bookmark folder doesn't do exactly the same thing as the arrow in the location bar, or any arrow on a random combo box. They *do* all mean "click this to drop down a list of choices".)
Since the popup lets you change search engines, which is most obviously indicated by the [G] changing to a Y! etc., I'm wondering why it wouldn't be best to keep that 'v' attached to the [G] rather than pulling it over to the right.
And since there's a '->' button outside the location bar (it's staying there, isn't it?), I'm also wondering why it wouldn't be best to put the '.O' button outside the search bar.
| Reporter | ||
Comment 6•20 years ago
|
||
(In reply to comment #5)
> Since the popup lets you change search engines, which is most obviously
> indicated by the [G] changing to a Y! etc., I'm wondering why it wouldn't be
> best to keep that 'v' attached to the [G] rather than pulling it over to the
> right.
The motivation behind this is that the current behavior isn't very discoverable; there aren't any other places I can think of in the FF UI where clicking an icon allows you to change it to another icon.
Google toolbar has made a similar switch recently (to the drop-down being on a button to the right of the search field), probably for similar reasons.
If we do continue to allow clicking the icon to open the drop-down, it would be for people who want to remove the new search "go" button from their toolbars via customization.
| Assignee | ||
Comment 7•20 years ago
|
||
> The motivation behind this is that the current behavior isn't very
> discoverable; there aren't any other places I can think of in the FF UI where
> clicking an icon allows you to change it to another icon.
That's a good goal, but I'm not convinced that a tiny arrow attached to a magnifying-glass icon will be any more discoverable than a tiny arrow attached to a search-engine icon. Although I suppose at least it's clearer that it's part of the chrome rather than part of the site's icon.
For that matter, do we have any data on how discoverable the little arrows on the back and forward buttons are? Maybe it's simply the case that tiny little arrows aren't part of the collective user consciousness and are easily overlooked.
| Reporter | ||
Comment 8•20 years ago
|
||
(In reply to comment #7)
> That's a good goal, but I'm not convinced that a tiny arrow attached to a
> magnifying-glass icon will be any more discoverable than a tiny arrow attached
> to a search-engine icon. Although I suppose at least it's clearer that it's
> part of the chrome rather than part of the site's icon.
Yes, and at the very least it's consistent with both the back/forward buttons, and with IE7's implementation of the same feature (which should be more discoverable to would-be switchers). (Note that while the Mac has some precedent for drop-downs attached to icons in the left interior of search boxes, that's not always the case on other platforms.)
> For that matter, do we have any data on how discoverable the little arrows on
> the back and forward buttons are? Maybe it's simply the case that tiny little
> arrows aren't part of the collective user consciousness and are easily
> overlooked.
Indeed--it's hard to say without better metrics and more user testing (both of which will hopefully be forthcoming). Do you have any alternate proposals for making this functionality more discoverable?
| Assignee | ||
Comment 9•20 years ago
|
||
> Do you have any alternate proposals for making this functionality
> more discoverable?
Only by making the arrow larger, but discoverability appears to conflict with perceived consistency there. I personally would probably trust in the broader notion of "clicking an arrow makes choices appear", but I understand the other side of that argument too.
Just to cover all the possibilities, what about keeping the arrow next to the icon, but making it bigger? Or for that matter, making it bigger wherever it might appear.
| Assignee | ||
Comment 10•20 years ago
|
||
Sorry, I should proofread my comments better. I meant to end by mentioning that there are currently three sizes of drop-down arrow on the Mac, and that the tiny ones could perhaps be bumped up to the medium size.
Comment 11•20 years ago
|
||
Why do the icon and the button have to be separate, anyway? I think that those can be naturally combined i.e. the button can be labelled with the icon. Just like IE7, only with a search engine icon on the button.
This would have couple of advantages:
- it would seem fairly intuitive to click on the icon to perform search with the associated engine. "Click on the Google icon to search Google"
- as already pointed out, it's the engine represented by an icon that you are changing from the drop-down. Therefore, it makes more sense for the drop-down to be attached to the icon. Placing it on the opposite side of the field isn't particularly logical and can potentially by confused with search history. Talking about consistency - similar drop-down in the location bar displays the field's history.
Are there any compelling arguments against the above solution?
Comment 12•20 years ago
|
||
(In reply to comment #5)
> And since there's a '->' button outside the location bar (it's staying there,
> isn't it?), I'm also wondering why it wouldn't be best to put the '.O' button
> outside the search bar.
It's staying outside, yes, but we're hoping to get it smacked up against the side so that it's more closely associated with the location bar. Similarly, the search button should be "attached" to the search field so that it is visually associated.
(In reply to comment #7)
> For that matter, do we have any data on how discoverable the little arrows on
> the back and forward buttons are? Maybe it's simply the case that tiny little
> arrows aren't part of the collective user consciousness and are easily
> overlooked.
No firm data, no, but they are pretty common widgets, especially when placed at the right of a text field. Remember, this is work being targetted at A2; we'll get better usage data based on that release, and can always make the change based on that. Let's not second-guess the entire compound button system just now. Your comments about the arrow size are well justified, though; we'll make sure to do that in the new default theme.
(In reply to comment #11)
> Why do the icon and the button have to be separate, anyway? I think that those
> can be naturally combined i.e. the button can be labelled with the icon. Just
> like IE7, only with a search engine icon on the button.
We do have data showing that people don't recognize the icons as the icons of the search engines, especially the [G] for the Google icon. The magnifying glass has become a well-recognized indicator of the "search" task. I don't know how much I care about which of the two are in the search field as opposed to being the label on the button, though. If people want the ASCII versions of these alternatives:
.---------------------------.---.
( [G] |O. v)
`---------------------------'---'
.---------------------------.----.
( O. |[G] v)
`---------------------------'----'
The latter actually parses better (for english speakers, anyway) as "Search for <term> with <engine>".
| Assignee | ||
Comment 13•20 years ago
|
||
(In reply to comment #12)
> .---------------------------.---.
> ( [G] |O. v)
> `---------------------------'---'
>
> .---------------------------.----.
> ( O. |[G] v)
> `---------------------------'----'
>
> The latter actually parses better (for english speakers, anyway) as
> "Search for <term> with <engine>".
Despite the natural-language syntax, I think the first of those is much clearer. People aren't used to thinking of site icons as buttons, so I like the less ambiguous magnifying-glass Search button, visually attached to the field. Whether the icon stays in the field at all is then a matter for bug 258672.
See attached screenshot. Of course, it still needs new icons to link the Go and Search buttons with their respective fields.
| Assignee | ||
Comment 14•20 years ago
|
||
How's this?
| Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Comment 15•20 years ago
|
||
(In reply to comment #13)
> Despite the natural-language syntax, I think the first of those is much
> clearer. People aren't used to thinking of site icons as buttons, so I like
> the less ambiguous magnifying-glass Search button, visually attached to the
> field.
While I agree, I think there is a problem with the icons in the dropdown not appearing under the icon in the field, since it is the icon that you are choosing, not the button.
(In reply to comment #14)
> Search button and drop-down at right
>
> How's this?
In your mockup, if the window is maximized, the dropdown has to appear in such position that the icons in it are not aligned with the button nor with the icon in the field. The dropdown would look detached and misaligned. In my mind, if the arrow is on the right, the icons in the dropdown should be aligned to the right as well, so they appear under the button, like in the mockup in comment #3.
| Assignee | ||
Comment 16•20 years ago
|
||
(In reply to comment #15)
> While I agree, I think there is a problem with the icons in the dropdown not
> appearing under the icon in the field, since it is the icon that you are
> choosing, not the button.
Well, really you're not choosing an icon at all, you're choosing a search provider. That's represented by an icon, true.
> In your mockup, if the window is maximized, the dropdown has to appear in such
> position that the icons in it are not aligned with the button nor with the icon
> in the field. The dropdown would look detached and misaligned. In my mind, if
> the arrow is on the right, the icons in the dropdown should be aligned to the
> right as well, so they appear under the button, like in the mockup in comment
> #3.
I'm of two minds about that. On the one hand, the icons are certainly strong identifiers for the providers, and putting them near where the mouse is going to be would be convenient and clear. On the other hand, nowhere else in the browser UI, or in UI in general that I can think of, are menus right-aligned with icons at the right. Funcitonal it may be, but it looks really bizarre.
Also bear in mind that that provider icon inside the field may not remain at all once there's grey text instead.
Comment 17•20 years ago
|
||
(In reply to comment #16)
> Well, really you're not choosing an icon at all, you're choosing a search
> provider. That's represented by an icon, true.
Sure, that's what I meant, sorry for being unclear.
> I'm of two minds about that. On the one hand, the icons are certainly strong
> identifiers for the providers, and putting them near where the mouse is going
> to be would be convenient and clear. On the other hand, nowhere else in the
> browser UI, or in UI in general that I can think of, are menus right-aligned
> with icons at the right. Functional it may be, but it looks really bizarre.
Yes, I feel the same way. I don't know, maybe not having the icons in the dropdown directly under the button is not a that bad after all?
Every single design brought up so far has some drawbacks. Perhaps we should implement what seems best right now and be ready to adjust it later based on the feedback. It's still something that can be changed in beta stage, isn't it?
> Also bear in mind that that provider icon inside the field may not remain at
> all once there's grey text instead.
I would oppose that. The grey text may be helpful for users that don't necessarily recognize the icons, but for those who know them, the icons are the best solution and the quickest way to identify the current engine. What's more, as the engine name in grey text is associated with a particular engine icon, more users would learn about what icons stand for, which would make them even more useful. So these may actually be complementary.
| Assignee | ||
Comment 18•20 years ago
|
||
(In reply to comment #17)
> Every single design brought up so far has some drawbacks. Perhaps we should
> implement what seems best right now and be ready to adjust it later based on
> the feedback. It's still something that can be changed in beta stage, isn't it?
Sounds reasonable to me. The attachment is a fully functional screenshot, not just a mock-up, so after giving this a weekend to congeal I'll pull it into a patch.
> The grey text may be helpful for users that don't
> necessarily recognize the icons, but for those who know them, the icons are the
> best solution and the quickest way to identify the current engine.
You may want to repeat that over in bug 258672.
| Assignee | ||
Comment 19•20 years ago
|
||
The UI may be subject to further change in later iterations, but here's a first pass at it.
Attachment #220410 -
Flags: ui-review?(mconnor)
Attachment #220410 -
Flags: review?(gavin.sharp)
| Assignee | ||
Comment 20•20 years ago
|
||
This image must be placed in browser/themes/{pinstripe|winstripe}/browser/
| Assignee | ||
Comment 21•20 years ago
|
||
Comment on attachment 220410 [details] [diff] [review]
Implements above screenshot, plus context menu
Some merge issues to be resolved. New patch coming.
Attachment #220410 -
Attachment is obsolete: true
Attachment #220410 -
Flags: ui-review?(mconnor)
Attachment #220410 -
Flags: review?(gavin.sharp)
| Assignee | ||
Comment 22•20 years ago
|
||
As before, but moves the button back inside the searchbox. Some of the less obvious changes support a context menu on the button.
Still needs the icon added to both themes directories.
Attachment #220181 -
Attachment is obsolete: true
Attachment #220741 -
Flags: ui-review?(beltzner)
Attachment #220741 -
Flags: review?(joe)
Comment 23•20 years ago
|
||
Comment on attachment 220741 [details] [diff] [review]
Patch with resolved merge and shifted button
Looks good. I need to think more about whether or not the context menu should be there -- initially it seems a little odd to replicate that function -- but I like the fact that it's available :)
Attachment #220741 -
Flags: ui-review?(beltzner) → ui-review+
| Assignee | ||
Comment 24•20 years ago
|
||
(In reply to comment #23)
> Looks good. I need to think more about whether or not the context menu should
> be there -- initially it seems a little odd to replicate that function -- but I
> like the fact that it's available :)
Other than the fact that it made sense to me to have it there, I like the fact that it makes a bigger target than the teeny little arrow, now that the main button does something else.
Comment 25•20 years ago
|
||
I remembered the other reason it should be in the binding, namely that if it isn't, its fun to add into existing customized toolbars (totally need a better solution for a lot of those problems)
| Reporter | ||
Comment 26•20 years ago
|
||
Since the button is now moved inside the search control, can you move most of the implementation into the searchbox binding?
| Assignee | ||
Comment 27•20 years ago
|
||
Moving everything into the searchbox binding means no context menu, due to bug 336662 (crashing bug when using the XUL that should accomplish that). Once that bug is fixed, it's a one-line change to put a context menu back in if we want one. Adding a context menu has been pulled out to bug 336667.
Or if we prefer, we can put the context menu back in by adding a <menupopup> definition to browser.xul, as was done in the previous patch.
Attachment #220741 -
Attachment is obsolete: true
Attachment #220851 -
Flags: review?(joe)
Attachment #220741 -
Flags: review?(joe)
| Assignee | ||
Updated•20 years ago
|
Attachment #220851 -
Flags: superreview?(bugs)
| Reporter | ||
Comment 28•20 years ago
|
||
Comment on attachment 220851 [details] [diff] [review]
Consolidated implementation; no context menu
Thanks, this'll be handy when it's done--a few minor comments follow:
+ handleSearchCommand: function BrowserSearch_handleSearchCommand(aEvent, aTextBox) {
Since AFAICT this is only getting called from the search widget, why not put this code in the
XBL?
+ if (textBox.value && !textBox.hasAttribute("disableautocomplete")) {
+ textBox._formHistSvc.addEntry(textBox.getAttribute(
+ "autocompletesearchparam"),
textBox.value);
+ }
Fix the weird line break/indentation here.
+ if (! textBox) {
+ var searchBar = this.getSearchBar();
+ if (! searchBar)
+ this.webSearch();
+ else
+ textBox = searchBar.textbox;
+ }
+
+ if (textBox) {
This logic could be a little clearer--can searchBar.textbox ever be null/undefined?
this.webSearch() wouldn't be called in that case. Also, since the last if clause takes over
the rest of the function, consider just returning if the textbox isn't set.
+ var evt = aEvent || textBox.mEnterEvent;
Could you add a comment about what textBox.mEnterEvent means in this case? Under what
conditions will aEvent be unset here?
- </xul:textbox>
+
+ <xul:textbox class="searchbar-textbox"
Watch the indentation here...
+ id="searchbar-textbox"
Why add this?
+ onpopupshowing="document.getElementById
('searchbar').rebuildPopup();"
+ oncommand="document.getElementById
('searchbar').onEnginePopupCommand(event); event.stopPropagation()">
+ oncommand="document.getElementById('searchbar').openManager
(event); event.stopPropagation()"/>
The searchbar element should be findable by navigating up the binding's tree, rather than by
going out to the document context--staying within the scope of the binding would give you
better encapsulation.
+ <property name="textbox"
+ onget="return this._textbox;"
+ readonly="true"/>
If you move the search function back into the binding (if it's possible--I don't know your
reasons), this wouldn't be necessary.
Other stuff looks good.
Attachment #220851 -
Flags: review?(joe) → review-
| Assignee | ||
Comment 29•20 years ago
|
||
Thanks. Changes made according to comments throughout; I'm only replying to bits that I have any questions about. In several cases, the issues you pointed out became moot when other changes were made.
> Fix the weird line break/indentation here.
I couldn't ever see a weird line break, but I shortened the lines. Hopefully that'll fix it.
> + if (! textBox) {
> + var searchBar = this.getSearchBar();
> + if (! searchBar)
> + this.webSearch();
> + else
> + textBox = searchBar.textbox;
> + }
> +
> + if (textBox) {
>
> This logic could be a little clearer--can searchBar.textbox ever be
> null/undefined?
Not that I know of, but when the function was pulled out into browser.xul, there was a risk that someone would call it with some other textbox, or none. Now that handleSearchCommand is part of search.xul, I'm much more willing to assume that the textbox being referred to is the one in that searchbox, and to dispense with the paranoid error checks.
> + var evt = aEvent || textBox.mEnterEvent;
>
> Could you add a comment about what textBox.mEnterEvent means in this case?
That was mistakenly taken from the function's original home in onTextEntered(). I've put it back where it belongs.
> + id="searchbar-textbox"
>
> Why add this?
That was so it could be found by ID. No longer needed.
> The searchbar element should be findable by navigating up the binding's tree,
> rather than by going out to the document context--staying within the scope
> of the binding would give you better encapsulation.
Constructions like
this.parentNode.parentNode.parentNode.parentNode.parentNode.openManager(event);
are pretty horrible. I've changed to that form -- but is there any more reasonable alternative, when you're five layers deep in a XUL tree?
Attachment #220851 -
Attachment is obsolete: true
Attachment #220877 -
Flags: superreview?(bugs)
Attachment #220877 -
Flags: review?(joe)
Attachment #220851 -
Flags: superreview?(bugs)
Comment 30•20 years ago
|
||
Comment on attachment 220877 [details] [diff] [review]
Patch addressing Joe's comments
>Index: components/search/content/search.xml
>+ <xul:toolbarbutton id="search-go-button"
>+ class="search-go-button"
>+ type="menu-button"
>+ anonid="searchbar-dropmarker"
>+ oncommand="this.parentNode.parentNode.parentNode
>+ .handleSearchCommand(event, null);"
null is no longer needed.
>+ <xul:menupopup anonid="searchbar-popup"
>+ class="searchbar-popup"
>+ position="after_end"
>+ onpopupshowing="this.parentNode.parentNode.parentNode
>+ .parentNode.rebuildPopup();"
Any reason this needs to happen each time the popup is shown? I tried to avoid rebuilding each time, since the search bar already observes changes and rebuilds only when necessary.
> const kXULNS =
> "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
>+
>+ // Prepend engines
nit: trailing whitespace
>+
>+ <method name="handleSearchCommand">
here too
>+ <parameter name="aEvent"/>
>+ <body><![CDATA[
>+ var textBox = this._textbox;
>+ if (textBox) {
Is there a reason for this check? _textbox should never be null, but if it is I think it would be better to find out with an exception rather than silent lack of functionality.
>+ // Save the current value in the form history
>+ if (textBox.value && !textBox.hasAttribute("disableautocomplete")) {
>+ textBox._formHistSvc.addEntry(textBox.getAttribute("autocompletesearchparam"),
>+ textBox.value);
>+ }
>+
>+ var newTab = textBox._prefBranch.getBoolPref("browser.search.openintab");
>+ var textValue = textBox.value;
>+ // Ignore greyed-out hint text in "empty" searchboxes.
>+ if (textBox.getAttribute("empty") == "true")
>+ textValue = null;
nit: use "" instead of null
| Assignee | ||
Comment 31•20 years ago
|
||
Fixed all the problems Gavin noticed; thanks.
> Any reason this needs to happen each time the popup is shown? I tried to avoid
> rebuilding each time, since the search bar already observes changes and
> rebuilds only when necessary.
I cheated a little. I know that updating the menu to show when a new search engine was found in a <link rel> tag on a page will be far more straightforward if it's done when the popup is shown, so I didn't revert that change. It's admittedly less efficient, but it should be both fairly uncommon and fast enough to be unnoticeable.
I missed removing the now-redundant rebuildPopup() from the browser-search-engine-modified observer method last time through; this patch corrects that.
Attachment #220877 -
Attachment is obsolete: true
Attachment #220883 -
Flags: superreview?(bugs)
Attachment #220883 -
Flags: review?(joe)
Attachment #220877 -
Flags: superreview?(bugs)
Attachment #220877 -
Flags: review?(joe)
| Assignee | ||
Comment 32•20 years ago
|
||
Gavin's convinced me that there's no need to rebuild the whole popup menu.
Attachment #220886 -
Flags: superreview?(bugs)
Attachment #220886 -
Flags: review?(joe)
| Assignee | ||
Updated•20 years ago
|
Attachment #220883 -
Attachment is obsolete: true
Attachment #220883 -
Flags: superreview?(bugs)
Attachment #220883 -
Flags: review?(joe)
| Reporter | ||
Comment 33•20 years ago
|
||
Comment on attachment 220886 [details] [diff] [review]
Addressing more of Gavin's comments
+ if (textBox.value && !textBox.hasAttribute("disableautocomplete")) {
+ textBox._formHistSvc.addEntry(textBox.getAttribute("autocompletesearchparam"),
+ textBox.value);
+ }
Is this going to run afoul of the gray search engine text? Maybe move the check for that above this, and use the textValue var in the test--then this conditional should handle the case.
+ oncommand="this.parentNode.parentNode.parentNode
+ .handleSearchCommand(event);"
+ oncommand="this.parentNode.parentNode.parentNode
+ .parentNode.onEnginePopupCommand(event);
+ event.stopPropagation()">
+ oncommand="this.parentNode.parentNode.parentNode
+ .parentNode.parentNode.openManager(event);
Can these use _getParentSearchbar() (or similar)?
| Assignee | ||
Comment 34•20 years ago
|
||
(In reply to comment #33)
> + if (textBox.value && !textBox.hasAttribute("disableautocomplete")) {
> Is this going to run afoul of the gray search engine text? Maybe move the
> check for that above this, and use the textValue var in the test--then this
> conditional should handle the case.
Done.
> + oncommand="this.parentNode.parentNode.parentNode
> + .handleSearchCommand(event);"
> Can these use _getParentSearchbar() (or similar)?
Moved them all into a handler, which allows direct access to the searchbox.
Attachment #220886 -
Attachment is obsolete: true
Attachment #220997 -
Flags: superreview?(bugs)
Attachment #220997 -
Flags: review?(joe)
Attachment #220886 -
Flags: superreview?(bugs)
Attachment #220886 -
Flags: review?(joe)
| Reporter | ||
Updated•20 years ago
|
Attachment #220997 -
Flags: review?(joe) → review+
Comment 35•20 years ago
|
||
Comment on attachment 220997 [details] [diff] [review]
Patch addressing Joe's comments
>Index: components/search/content/search.xml
>+ <xul:toolbarbutton id="search-go-button"
You shouldn't use ids inside XBL bindings, since ids are supposed to be unique per-document and you could theoretically have N instances of a particular binding. Use .class instead.
>+#search-go-button {
.search-go-button
One more patch.
Attachment #220997 -
Flags: superreview?(bugs) → superreview-
| Assignee | ||
Comment 36•20 years ago
|
||
Attachment #221008 -
Flags: superreview?(bugs)
| Assignee | ||
Updated•20 years ago
|
Attachment #220997 -
Attachment is obsolete: true
Comment 37•20 years ago
|
||
Comment on attachment 221008 [details] [diff] [review]
Patch with more class (see Ben's comments)
classy!
sr=ben@mozilla.org
Attachment #221008 -
Flags: superreview?(bugs) → superreview+
Comment 38•20 years ago
|
||
Attachment #221008 -
Attachment is obsolete: true
Comment 39•20 years ago
|
||
fixed-on-trunk, fixed-1.8-branch
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 40•20 years ago
|
||
Am I right to assume that the theme work for FF 2.0 will make the new search box button (moved to the right) not look sucky, like maybe just a colored magnifying glass with a transparent background instead of a gray box?
~B
Comment 41•20 years ago
|
||
Argh... forgot to mention... This bug appears to have regressed the padding on "Manage Search Engines..." in the search engine drop down. It's no longer aligned vertically with the search engine name.
~B
| Assignee | ||
Comment 42•20 years ago
|
||
(In reply to comment #41)
> Argh... forgot to mention... This bug appears to have regressed the padding on
> "Manage Search Engines..." in the search engine drop down. It's no longer
> aligned vertically with the search engine name.
Unless I misunderstand or it's a very subtle difference, it looks right to me on Mac and Linux. What platform are you using?
Comment 43•20 years ago
|
||
(In reply to comment #42)
> Unless I misunderstand or it's a very subtle difference, it looks right to me
> on Mac and Linux. What platform are you using?
>
I can see it on Windows, see attached screenshot.
Comment 44•20 years ago
|
||
Not that I see what your talking about but create a new bug, let this fixed bug die.
| Assignee | ||
Comment 45•20 years ago
|
||
(In reply to comment #41)
> This bug appears to have regressed the padding on
> "Manage Search Engines..." in the search engine drop down. It's no longer
> aligned vertically with the search engine name.
Filed as bug 337174.
| Assignee | ||
Comment 46•20 years ago
|
||
(In reply to comment #40)
> Am I right to assume that the theme work for FF 2.0 will make the new search
> box button (moved to the right) not look sucky, like maybe just a colored
> magnifying glass with a transparent background instead of a gray box?
It won't be a nice colored magnifying glass in the winstripe theme until the new images in bug 337131 are checked in, but it should currently be a greyscsale magnifying glass, not a grey box. Are you using a nightly build? If you're building yourself instead, perhaps your source update missed getting the new icon files?
Comment 47•19 years ago
|
||
Not sure we want/need/have the spiritual fortitude and energy required/ to re-open this bug, but some comments I've heard (mostly from people on Mac, fwiw) about this change from people in the HCI community whose opinions I trust include:
* it's strange that the drop-down isn't spatially associated with the thing that it changes; that is to say, it's on the right, but the icon/search engine name is on the left
* the tooltip reads "Search <name>" even if you hover on the arrow portion (that's probably a separate bug, and likely one we can't fix easily)
I'm not sure that these comments necessarily sway me to thinking the current design is wrong, but I wanted to put them out there to see if anyone else had reconsidered things after a few weeks of using the new search field.
| Assignee | ||
Comment 48•19 years ago
|
||
> * it's strange that the drop-down isn't spatially associated with the thing
> that it changes; that is to say, it's on the right, but the icon/search engine
> name is on the left
Having the menu open to the left, so the icons line up, would probably help that. That's bug 336868, which needs bug 335872 fixed to be fixed right.
> * the tooltip reads "Search <name>" even if you hover on the arrow portion
> (that's probably a separate bug, and likely one we can't fix easily)
I'm pretty sure it's not a trivial fix. It might be possible with the button and arrow split into two independent buttons, but I'm not sure what impact that will have on the display in other themes.
> I'm not sure that these comments necessarily sway me to thinking the
> current design is wrong, but I wanted to put them out there to see if anyone
> else had reconsidered things after a few weeks of using the new search field.
I agree that the current design isn't as obvious as we'd like it to be. But I can't think of one that's any clearer. :/
Comment 49•19 years ago
|
||
(In reply to comment #48)
> > * the tooltip reads "Search <name>" even if you hover on the arrow portion
> > (that's probably a separate bug, and likely one we can't fix easily)
>
> I'm pretty sure it's not a trivial fix.
It isn't, and it wasn't the only problem with the tooltips, so all search bar tooltips were removed in bug 311653.
You need to log in
before you can comment on or make changes to this bug.
Description
•