582121 - Web Console filter box should look like a search box

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Patrick Walton (:pcwalton)]

Attachment: Proposed patch, part 1.

The filter box on the Web Console should look like a search box on the underlying OS. (Because it doesn't actually do live search, but rather filters incoming messages, I'm leaving the magnifying glass icon out. However, the functionality is conceptually very similar, and so IMHO it should be styled in a way that suggests searching.)

Because this is an important part of the UX for the Firefox 4 Web Console, I'm requesting that the drivers consider betaN blocking status for this bug.

The attached patch is the first step toward implementing this feature, which is to make the search box rounded on the Mac. It also surrounds the filter box in a containing XUL stack so that we can move the "Clear Filter" button inside the box (like the native search boxes on Windows and Mac). The CSS rules are based on those of the Find bar.

Comment 1

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 460387 [details] [diff] [review]
Proposed patch, part 1.

this looks like a good start. Obviously we'll need similar styling for gnome and winstripe for this to get reviewed.

Seconding the request for blocking.

Comment 2

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

Can you not use a <textbox type=search>? That has effects other than styling, but I'm not sure whether they matter here.

Comment 3

[Patrick Walton (:pcwalton)]

(In reply to comment #1)
> Comment on attachment 460387 [details] [diff] [review]
> Proposed patch, part 1.
> 
> this looks like a good start. Obviously we'll need similar styling for gnome
> and winstripe for this to get reviewed.

Actually, Windows and Linux don't need styling tweaks for search boxes. They look OS native already.

(In reply to comment #2)
> Can you not use a <textbox type=search>? That has effects other than styling,
> but I'm not sure whether they matter here.

Unfortunately, this adds a magnifying glass, and per ddahl's feedback I wanted to leave that out, because it suggests real-time search instead of filtering incoming messages. (Might be nice to have a funnel icon.)

Comment 4

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

Using <textbox type="search"> and hiding the magnifying glass seems to me like it would be easier than styling everything manually.

Comment 5

[Patrick Walton (:pcwalton)]

It doesn't look like it's possible to hide the magnifying glass for a search field without modifying the platform. Search fields are implemented on the Mac as native NSSearchFieldCells [1]. http://mxr.mozilla.org/mozilla-central/source/widget/src/cocoa/nsNativeThemeCocoa.mm#214). To remove the magnifying glass, the button cell needs to be removed using the method -[NSSearchFieldCell setSearchButtonCell: nil] [2]. But this method isn't used anywhere in the platform.

[1]: http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/SearchFields/Articles/CustomizingSearchFields.html#//apple_ref/doc/uid/20002246-BABFIBIA
[2]: http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/SearchFields/Articles/CustomizingSearchFields.html#//apple_ref/doc/uid/20002246-BABFIBIA

Comment 6

[Johnathan Nightingale [:johnath]]

FWIW, on mac, fields which act as string-matching filters (e.g. the filter for Console.app) use the magnifying glass as well. IOW, it might be platform-acceptable to leave this on mac, and hide it elsewhere.

Ask Limi!

Comment 7

[Patrick Walton (:pcwalton)]

The Filter box in Console.app is actually a live search; it filters messages that have already been logged. What makes our box different is that it filters only *incoming* messages.

(IMO the behavior should be changed to be more like Console.app, so that the filter bar filters all messages, not just incoming ones. That would require some significant reworking of the console internals, however. I've been trying to keep the UI tweaks small and simple, in order to maximize the chance that they land in Firefox 4.)

Limi has been cc'd on the bug.

Comment 8

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #7)
> The Filter box in Console.app is actually a live search; it filters messages
> that have already been logged. What makes our box different is that it filters
> only *incoming* messages.

really? I expected it to work like the filter in Console.app which would change the contents of the console depending on the filter terms that were entered. You're going to want to use this to track down specific items in the logs so that seems like pretty important behavior.
 
> (IMO the behavior should be changed to be more like Console.app, so that the
> filter bar filters all messages, not just incoming ones. That would require
> some significant reworking of the console internals, however. I've been trying
> to keep the UI tweaks small and simple, in order to maximize the chance that
> they land in Firefox 4.)

mos'def.

Comment 9

[David Dahl :ddahl]

(In reply to comment #8)

> really? I expected it to work like the filter in Console.app which would change
> the contents of the console depending on the filter terms that were entered.
> You're going to want to use this to track down specific items in the logs so
> that seems like pretty important behavior.

well, we had to punt on that in favor of more fundamental work, it should not be that hard to get working... however we need to figure out log truncation and log searching.

Comment 10

[Patrick Walton (:pcwalton)]

Attachment: Mac screenshot.

Here's a screenshot of what I'm proposing on the Mac.

Comment 11

[Patrick Walton (:pcwalton)]

Attachment: Proposed patch, part 1 (trunk rebase 2010-07-30).

Patch rebased to trunk.

Comment 12

[Patrick Walton (:pcwalton)]

Attachment: Proposed patch, version 2.

Per review comments I'm going to simply make this a search box. New patch attached.

Comment 13

[Patrick Walton (:pcwalton)]

See bug 583359 for making the log history searchable.

Comment 14

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

Comment on attachment 461675 [details] [diff] [review]
Proposed patch, version 2.

You should also remove stringFilterClear from headsUpDisplay.properties since this removes its only use. r=me with that.

Comment 15

[David Dahl :ddahl]

Comment on attachment 461675 [details] [diff] [review]
Proposed patch, version 2.

if gavin digs it, I dig it.

Comment 16

[Patrick Walton (:pcwalton)]

Attachment: Proposed patch, version 2 (with review comments).

Comment 17

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

Comment on attachment 461683 [details] [diff] [review]
Proposed patch, version 2 (with review comments).

a=me

Comment 18

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/mozilla-central/rev/a731dc8d3b27