Closed Bug 582121 Opened 11 years ago Closed 11 years ago

Web Console filter box should look like a search box

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pcwalton, Assigned: pcwalton)

References

Details

Attachments

(2 files, 3 obsolete files)

Attached patch Proposed patch, part 1. (obsolete) — Splinter Review
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.
Attachment #460387 - Flags: feedback?(rcampbell)
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.
Attachment #460387 - Flags: feedback?(rcampbell) → feedback+
Can you not use a <textbox type=search>? That has effects other than styling, but I'm not sure whether they matter here.
(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.)
Using <textbox type="search"> and hiding the magnifying glass seems to me like it would be easier than styling everything manually.
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
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!
Attachment #460387 - Flags: review?(gavin.sharp)
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.
Attachment #460387 - Flags: ui-review?(limi)
(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.
(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.
Blocks: consoleui
Keywords: uiwanted
Attached image Mac screenshot.
Here's a screenshot of what I'm proposing on the Mac.
Patch rebased to trunk.
Attachment #460387 - Attachment is obsolete: true
Attachment #461672 - Flags: review?(gavin.sharp)
Attachment #461672 - Flags: feedback?(rcampbell)
Attachment #460387 - Flags: ui-review?(limi)
Attachment #460387 - Flags: review?(gavin.sharp)
Attached patch Proposed patch, version 2. (obsolete) — Splinter Review
Per review comments I'm going to simply make this a search box. New patch attached.
Attachment #461672 - Attachment is obsolete: true
Attachment #461675 - Flags: review?(gavin.sharp)
Attachment #461675 - Flags: feedback?(ddahl)
Attachment #461672 - Flags: review?(gavin.sharp)
Attachment #461672 - Flags: feedback?(rcampbell)
See bug 583359 for making the log history searchable.
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.
Attachment #461675 - Flags: review?(gavin.sharp) → review+
Attachment #461675 - Flags: approval2.0?
Comment on attachment 461675 [details] [diff] [review]
Proposed patch, version 2.

if gavin digs it, I dig it.
Attachment #461675 - Flags: feedback?(ddahl) → feedback+
Attachment #461675 - Attachment is obsolete: true
Attachment #461675 - Flags: approval2.0?
Comment on attachment 461683 [details] [diff] [review]
Proposed patch, version 2 (with review comments).

a=me
Attachment #461683 - Flags: approval2.0? → approval2.0+
Whiteboard: [needs-commit]
Whiteboard: [needs-commit] → [checkin-needed]
Keywords: checkin-needed
Whiteboard: [checkin-needed]
https://hg.mozilla.org/mozilla-central/rev/a731dc8d3b27
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
blocking2.0: ? → ---
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.