Web Console filter box should look like a search box

RESOLVED FIXED

Status

RESOLVED FIXED
9 years ago
6 months ago

People

(Reporter: pcwalton, Assigned: pcwalton)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 460387 [details] [diff] [review]
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.
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.
(Assignee)

Comment 3

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

Comment 5

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

Updated

9 years ago
Attachment #460387 - Flags: review?(gavin.sharp)
(Assignee)

Comment 7

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

Updated

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

Comment 9

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

Updated

9 years ago
Blocks: 559481
(Assignee)

Updated

9 years ago
Keywords: uiwanted
(Assignee)

Comment 10

9 years ago
Created attachment 461671 [details]
Mac screenshot.

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

Comment 11

9 years ago
Created attachment 461672 [details] [diff] [review]
Proposed patch, part 1 (trunk rebase 2010-07-30).

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

Comment 12

9 years ago
Created attachment 461675 [details] [diff] [review]
Proposed patch, version 2.

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

Comment 13

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

Updated

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

Comment 16

9 years ago
Created attachment 461683 [details] [diff] [review]
Proposed patch, version 2 (with review comments).
Attachment #461683 - Flags: approval2.0?
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+

Updated

9 years ago
Whiteboard: [needs-commit]

Updated

9 years ago
Whiteboard: [needs-commit] → [checkin-needed]

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [checkin-needed]
https://hg.mozilla.org/mozilla-central/rev/a731dc8d3b27
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed, uiwanted
Resolution: --- → FIXED
blocking2.0: ? → ---

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.