393718 - Style searchbar splitter nicely

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect, P3)

Description

[aja+bugzilla]

Searchbar's splitter looks bad since landing of bug 373696 (which styles toolbarbar separators natively).

Comment 2

[Jonathan Haas]

it looked bad even before landing of bug 373696.

Comment 3

[aja+bugzilla]

FWIW, "Separator" would look better styled natively as well.
Want a separate bug?

Comment 4

[Michael Ventnor]

I have something which will make the splitter look a lot better and more native. If I post it here now would it get M9 approval?

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #3)
> FWIW, "Separator" would look better styled natively as well.
> Want a separate bug?

The separator is style natively on trunk (attachment 278239 [details] [diff] [review]).

(In reply to comment #4)
> I have something which will make the splitter look a lot better and more
> native. If I post it here now would it get M9 approval?

Depends on how good your solution is ;)

Comment 6

[Michael Ventnor]

Attachment: splitter-ew.png

I think this is the style that Windows uses for toolbar splitters. This was made in the GIMP using very transparent black, so it should fit in with any theme and still look "punctured" in.

This goes in toolkit/themes/winstripe/global/splitter/.

Comment 7

[Michael Ventnor]

Attachment: splitter-ns.png

This goes in the same folder.

Comment 8

[Michael Ventnor]

Attachment: Patch

This uses the images above, so it takes the same approach as the Pinstripe splitter. But I added an extra class to Winstripe since the current solid line is still needed for places other than the toolbar, such as the sidebar.

Comment 9

[Michael Ventnor]

BTW, the images follow the style of Office and IE on their toolbars.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 283952 [details]
splitter-ew.png

This won't work with a dark background. I also don't think the CSS belongs into toolkit or that this is generally a good fix (as in: it doesn't style splitters natively, which would involve -moz-appearance), but I'll leave that to mano.

Comment 11

[Asaf Romano (gone)]

Comment on attachment 283954 [details] [diff] [review]
Patch

whathesaid, except the style rules will likely end up in toolkit's toolbar.css.

Comment 12

[Michael Ventnor]

(In reply to comment #10)
> (From update of attachment 283952 [details])
> This won't work with a dark background. I also don't think the CSS belongs into
> toolkit or that this is generally a good fix (as in: it doesn't style splitters
> natively, which would involve -moz-appearance), but I'll leave that to mano.
> 

Pinstripe uses images as well instead of a native style. I don't know Windows widgets and I don't think GTK has an equivalent of this.

Comment 13

[Dão Gottwald [:dao]]

Pinstripe doesn't have to support different color schemes, as far as I know. Your images will just disappear with a black high-contrast theme, for example. Maybe use something like |border-left: 3px dotted ThreeDShadow| instead?

Comment 14

[Michael Ventnor]

(In reply to comment #11)
> (From update of attachment 283954 [details] [diff] [review])
> whathesaid, except the style rules will likely end up in toolkit's toolbar.css.
> 

If it is put there, wouldn't the splitter on the customize dialog not pick up the styles?

Comment 15

[Wesley Johnston (:wesj)]

Attachment: Screenshot of a themed splitter on XP Classic

I styled this up with userchrome on my home machine to be just two pixels wide, with a 1px left border threedshadow, and 1 px right border threedhighlight (and a few extra pixels of margin to increase the hover target... not sure if that actually works or not), but the results work fine here. Screenshot attached.

Comment 16

[Michael Ventnor]

Attachment: Patch 2

This uses a dotted border approach. I can't put the rules in toolbar.css because the splitter in the toolbar doesn't pick it up for some reason.

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #15)
> Created an attachment (id=284119) [details]
> Screenshot of a themed splitter on XP Classic
> 
> I styled this up with userchrome on my home machine to be just two pixels wide,
> with a 1px left border threedshadow, and 1 px right border threedhighlight (and
> a few extra pixels of margin to increase the hover target... not sure if that
> actually works or not), but the results work fine here. Screenshot attached.

This looks like a normal separator.

Comment 18

[aja+bugzilla]

Attachment: another possible splitter look

Comment 19

[Dão Gottwald [:dao]]

Comment on attachment 284120 [details] [diff] [review]
Patch 2

>+splitter.dotted {
>+  min-width: 3px;
>+  margin: 3px 0;
>+  border: 0;
>+  border-left: 2px dotted ThreeDShadow;
>+}

This looks wrong. I think you want the width to be exactly zero.

Comment 20

[aja+bugzilla]

Or perhaps the following with a 1px wide transparent background image

border-left:  3px dotted ButtonShadow;
border-right: 3px dotted ButtonShadow;

This was always visible in the testing I've done -- default XP theme with default (blue), olive green, and silver color schemes, and b/w and w/b.

It gives two columns of dots (staggered at most heights), like the XP taskbar's splitters have.

FWIW, whatever the color effect used on the status line's grippy's dots is looks even better to me than ButtonShadow.

Comment 21

[Michael Ventnor]

(In reply to comment #19)
> (From update of attachment 284120 [details] [diff] [review])
> >+splitter.dotted {
> >+  min-width: 3px;
> >+  margin: 3px 0;
> >+  border: 0;
> >+  border-left: 2px dotted ThreeDShadow;
> >+}
> 
> This looks wrong. I think you want the width to be exactly zero.
> 

Really? It seems to me that min-width includes the border, from some testing I did with my cursor. Plus we mustn't forget Fitts' Law...

Comment 22

[Dão Gottwald [:dao]]

(In reply to comment #21)
> (In reply to comment #19)
> > This looks wrong. I think you want the width to be exactly zero.
> 
> Really? It seems to me that min-width includes the border, from some testing I
> did with my cursor.

It does, but since the width should be zero (otherwise you have an asymmetric padding), the min-width shouldn't be bigger than the border width.

> Plus we mustn't forget Fitts' Law...

Yet I don't think asymmetric padding is the way to go. You could create a theme-specific binding to add padding on both sides, but we want to avoid that for performance reasons. So I guess I would go without any padding, but maybe use a border width of 3px.

Comment 23

[Michael Ventnor]

(In reply to comment #22)
> (In reply to comment #21)
> > (In reply to comment #19)
> > > This looks wrong. I think you want the width to be exactly zero.
> > 
> > Really? It seems to me that min-width includes the border, from some testing I
> > did with my cursor.
> 
> It does, but since the width should be zero (otherwise you have an asymmetric
> padding), the min-width shouldn't be bigger than the border width.
> 
> > Plus we mustn't forget Fitts' Law...
> 
> Yet I don't think asymmetric padding is the way to go. You could create a
> theme-specific binding to add padding on both sides, but we want to avoid that
> for performance reasons. So I guess I would go without any padding, but maybe
> use a border width of 3px.
> 

If I do that then the splitter will no longer look familiar. The toolbar splitters of other Windows apps have 2px dots.

Comment 24

[Dão Gottwald [:dao]]

It doesn't look familiar to me either way. Toolbar splitters are usually attached to a toolbar border.

Comment 25

[Michael Ventnor]

(In reply to comment #24)
> It doesn't look familiar to me either way. Toolbar splitters are usually
> attached to a toolbar border.
> 

But it was the closest thing I could find as a consistent design for a toolbar splitter.

Comment 26

[Dão Gottwald [:dao]]

The Windows splitters don't even work similarly. Maybe we should abandon the idea to style the search bar splitter natively, and chose a layout that works best for us.

I like this one:

border-style: dotted;
border-color: ThreeDShadow;
border-width: 0 1px;
margin: 3px 0;
min-width: 4px;

Comment 27

[Michael Ventnor]

I don't think two lines is very suitable. The user would probably think its a flexible space between, and each line is used to expand/shrink it. At least thats what I would think. Or they could probably think its an empty toolbar inside.

IMO, the 2px dotted approach is best.

Comment 28

[Mike Connor [:mconnor]]

Let's go with this for now, I don't think we're going to have the splitter visible in non-customizing mode past b1 anyway so I don't think this needs to be final.

Comment 29

[Neil Deakin]

Comment on attachment 284120 [details] [diff] [review]
Patch 2


>+  templateNode.className = "dotted";

If you're going to use a class here, it should have a name like 'toolbar-splitter', not 'dotted', so it doesn't imply that it has to appear in a specific way, which won't be true on other platforms.

Comment 30

[Dão Gottwald [:dao]]

(In reply to comment #28)
> Let's go with this for now, I don't think we're going to have the splitter
> visible in non-customizing mode past b1 anyway so I don't think this needs to
> be final.

In the customizing mode, wouldn't the current splitter actually do a good job?

Comment 31

[Michael Ventnor]

Attachment: Change class name

Don't assume the splitter will take on a dotted look.

Comment 32

[Dão Gottwald [:dao]]

Comment on attachment 285159 [details] [diff] [review]
Change class name

>+splitter.toolbar-splitter {
>+  min-width: 3px;
>+  margin: 3px 0;
>+  border: 0;
>+  border-left: 2px dotted ThreeDShadow;
>+}

http://developer.mozilla.org/en/docs/Writing_Efficient_CSS#Don.27t_qualify_class-categorized_rules_with_tag_names

Comment 33

[Michael Ventnor]

Attachment: Acceptable CSS

Can you get the review done very soon please, Mano?

Comment 34

[Michael Ventnor]

Comment on attachment 285210 [details] [diff] [review]
Acceptable CSS

Mano might not have enough time... mconnor if you ever have time before Mano gets to review this can you do it?

Comment 35

[Asaf Romano (gone)]

Where's the orient attribute for a .toolbar-splitter set?

Comment 36

[Michael Ventnor]

I guess I did that for consistency, someone might want it in the future.

Comment 37

[Dão Gottwald [:dao]]

For vertical toolbars? ;)

One more thing:
(In reply to comment #28)
> Let's go with this for now, I don't think we're going to have the splitter
> visible in non-customizing mode past b1 anyway so I don't think this needs to
> be final.

The current development (bug 400327) looks like the splitter could eventually remain in the non-customizing mode.

Comment 38

[Michael Ventnor]

(In reply to comment #37)
> For vertical toolbars? ;)

It was a leftover from the original "dotted" class :)
It doesn't have to be for a toolbar, but I guess you're right.

Comment 39

[Michael Ventnor]

Attachment: New patch

Comment 40

[Asaf Romano (gone)]

Comment on attachment 285931 [details] [diff] [review]
New patch

s/border-left/-moz-border-start/
s/border: 0/border: none

r=mano otherwise.

Comment 41

[Michael Ventnor]

Attachment: For checkin

This patch is meant to improve the look of the thick and ugly splitter for Winstripe users. The reason I'm asking for M9 is because this patch is mostly no-risk CSS and this will fix the inevitable flood of complaints about this high-exposure splitter should it keep its current look into our first beta.

Comment 42

[Mike Connor [:mconnor]]

Comment on attachment 285983 [details] [diff] [review]
For checkin

this is an M9 blocker, doesn't need approval, but just so there's no confusion...

Comment 43

[Reed Loden [:reed]]

Checking in toolkit/content/customizeToolbar.js;
/cvsroot/mozilla/toolkit/content/customizeToolbar.js,v  <--  customizeToolbar.js
new revision: 1.37; previous revision: 1.36
done
Checking in toolkit/content/widgets/toolbar.xml;
/cvsroot/mozilla/toolkit/content/widgets/toolbar.xml,v  <--  toolbar.xml
new revision: 1.38; previous revision: 1.37
done
Checking in toolkit/themes/winstripe/global/splitter.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/splitter.css,v  <--  splitter.css
new revision: 1.6; previous revision: 1.5
done

Comment 44

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102504 Minefield/3.0a9pre. The splitter nows looks like the screenshot in https://bugzilla.mozilla.org/attachment.cgi?id=284141.

Comment 45

[Alex Faaborg [:faaborg] (Firefox UX)]

It's great that we got this cleaned up for M9, but I'm Reopening for M10 since this didn't go through ui-r for the first iteration and this design appears in a very high visibility interface. I'll follow up with a suggested design after we have made a little more progress in determining the planned control scheme and I've had a chance to get beltzner's input.

My initial take on the issue is that the small circle works well in the new mac theme, but we should not have any affordance on windows (outside of a cursor change on hover) to reduce overall visual complexity.  Discoverability should be proportional to how much something actually needs to be discovered.  Discoverability is also a zero-sum game, every affordance we add reduces the discoverability of surrounding affordances.

Comment 46

[Dão Gottwald [:dao]]

Attachment: toolbar-splitter.png

This is taken from the current Mac theme, not the new one. It would work well for all kinds of toolbar backgrounds.

I do however think we shouldn't always show the splitter as it's not generally needed. If we follow bug 393733, I would suggest a different, less subtle appearance.

Comment 47

[Dão Gottwald [:dao]]

Attachment: patch for previous image

Comment 48

[Dão Gottwald [:dao]]

Attachment: screenshot

Comment 49

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

Comment on attachment 288153 [details]
screenshot

I think this is fine, but want to give Alex a chance to weigh in as he's directing the theme work for Firefox 3

Comment 50

[Dão Gottwald [:dao]]

Attachment: another screenshot

That's the native appearance on Ubuntu. We could imitate this on Windows.

Comment 51

[Reed Loden [:reed]]

(In reply to comment #50)
> Created an attachment (id=295445) [details]
> another screenshot
> 
> That's the native appearance on Ubuntu. We could imitate this on Windows.

What do you mean by "native appearance on Ubuntu"? This screenshot isn't what I see on Ubuntu for current trunk.

Comment 52

[Dão Gottwald [:dao]]

(In reply to comment #51)
> What do you mean by "native appearance on Ubuntu"?

-moz-appearance: splitter

Comment 53

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 288153 [details]
screenshot

I personally believe that we should remove the affordance entirely for simplicity.  This isn't a feature that needs to be discoverable, but if you happen to know about it or notice the cursor change on hover, it's nice to have.  This is one area where we can even pull ahead of Safari in terms of clean design.

Comment 54

[Dão Gottwald [:dao]]

I've been in favor of bug 393733, but making the splitter invisible sounds like an interesting idea, actually... It's also the easiest thing to do.

Comment 55

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 295445 [details]
another screenshot

While consistent with the visual affordance for resizing on GTK splitters and the GNOME panel and window list, I still think this is visually too heavy.  Is there any way we could get this affordance to appear only when the customization pallet is being displayed?  I'm pretty sure that is the only situation where I personally want it to be visible.  Even though it is a platform convention, the grippy part is just striking me as feeling more Netscape 4.x than Firefox 1.

Comment 56

[Dão Gottwald [:dao]]

(In reply to comment #55)
> Is there any way we could get this affordance to appear only when the
> customization pallet is being displayed?

Visually, yes, but I suppose making it functional in the customizing mode isn't trivial.

Comment 57

[Dão Gottwald [:dao]]

Attachment: remove the affordance

Comment 58

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

Comment on attachment 295517 [details] [diff] [review]
remove the affordance

>Index: browser/themes/gnomestripe/browser/browser.css

>+/*XXX The adjacent selector isn't working reliably

Is there a bug filed on that?

Comment 59

[Dão Gottwald [:dao]]

I think it's known, but I couldn't find a bug.

Comment 60

[Jonathan Haas]

I suppose it's bug 229915

Comment 61

[Dão Gottwald [:dao]]

Yeah, that's the one. Now that I know the bug, I can actually hack around it.

Comment 62

[Dão Gottwald [:dao]]

Attachment: remove the affordance v2

Comment 63

[Asaf Romano (gone)]

Hrm, so, why are you changing splitter.css? Other consumers may still want this. Also, what's the resize-interaction like after this change? would it "start" at some random point left to the search-bar?

Comment 64

[Asaf Romano (gone)]

Or you should keep the toolbar-splitter class set and claim in splitter.css that the element is invisible... What do other windows applications do here?

Comment 65

[Dão Gottwald [:dao]]

(In reply to comment #63)
> Hrm, so, why are you changing splitter.css?

The style was set in splitter.css because the class was set in toolkit code. That's not the case anymore.

> Other consumers may still want this.

Well, I don't see other consumers. Also, the styling wasn't ui-reviewed, which is why Alex reopened this bug.

> Also, what's the resize-interaction like after this change? would it
> "start" at some random point left to the search-bar?

Right at the edge.

(In reply to comment #64)
> Or you should keep the toolbar-splitter class set and claim in splitter.css
> that the element is invisible... What do other windows applications do here?

I don't know other applications with a toolbar splitter. If there are any, they are probably not using an OS widget, because (as far as I know) there is none.

Comment 66

[Dão Gottwald [:dao]]

(In reply to comment #65)
> (In reply to comment #64)
> > Or you should keep the toolbar-splitter class set and claim in splitter.css
> > that the element is invisible... What do other windows applications do here?
> 
> I don't know other applications with a toolbar splitter.

Oh, there's Safari 3 for Windows, but obviously it doesn't even try to look native.

Comment 67

[Alex Faaborg [:faaborg] (Firefox UX)]

>I don't know other applications with a toolbar splitter. If there are any, they
>are probably not using an OS widget, because (as far as I know) there is none.

We still see splitters in cases like when you want to modify the toolbars on your task bar in windows, but the task bar now needs to be unlocked first, since user's were accidently resizing things without intending to.  Since you can't actually drag something out of our toolbar hopefully this will be less of an issue, but I still wonder how often people will resize the search field by accident.

Comment 68

[Dão Gottwald [:dao]]

(In reply to comment #67)
> >I don't know other applications with a toolbar splitter. If there are any, they
> >are probably not using an OS widget, because (as far as I know) there is none.
> 
> We still see splitters in cases like when you want to modify the toolbars on
> your task bar in windows, but the task bar now needs to be unlocked first

These splitters as well as the Explorer ones are different in that they are there to resize (and move) toolbars, not toolbar elements.

Comment 69

[Alex Faaborg [:faaborg] (Firefox UX)]

>These splitters as well as the Explorer ones are different in that they are
>there to resize (and move) toolbars, not toolbar elements.

Yeah, I can't recall any controls to resize toolbar elements.  Let's drop the visual affordance on windows as well to get a cleaner look.

Comment 70

[Asaf Romano (gone)]

Comment on attachment 295672 [details] [diff] [review]
remove the affordance v2

r=mano code wise, this hurts usability IMO though :(

Comment 71

[Dão Gottwald [:dao]]

FWIW, I've been using this for the past few days on Windows and I think it works well.

Comment 72

[Reed Loden [:reed]]

Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.152; previous revision: 1.151
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.104; previous revision: 1.103
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.146; previous revision: 1.145
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.924; previous revision: 1.923
done
Checking in toolkit/themes/winstripe/global/splitter.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/splitter.css,v  <--  splitter.css
new revision: 1.7; previous revision: 1.6
done

Comment 73

[Spencer Selander [greenknight]]

(In reply to comment #65)

> 
> I don't know other applications with a toolbar splitter. If there are any, they
> are probably not using an OS widget, because (as far as I know) there is none.
> K-Meleon uses a whole bunch of splitters on their toolbars, which look very similar to the one that this bug is about.  

Comment 74

[Dão Gottwald [:dao]]

K-Meleon's splitters are similar to those of Windows Explorer.

Comment 75

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

With Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008011105 Minefield/3.0b3pre there isn't a splitter anymore. Got it removed somewhere else?

Comment 76

[Reed Loden [:reed]]

(In reply to comment #75)
> With Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre)
> Gecko/2008011105 Minefield/3.0b3pre there isn't a splitter anymore. Got it
> removed somewhere else?

It's invisible. :(

I preferred the old one...

Comment 77

[Jesse Ruderman]

In the Mac default theme, the address bar and search bar are now too close together.

Comment 78

[Dão Gottwald [:dao]]

(In reply to comment #77)
Last I heard the new Mac theme should land for beta 3, so I'm not sure if tweaking this now is worthwhile. Note that the width of the splitter can be anything above 8 px.

Comment 79

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

(In reply to comment #78)
> Last I heard the new Mac theme should land for beta 3, so I'm not sure if
> tweaking this now is worthwhile. Note that the width of the splitter can be
> anything above 8 px.

Dao, I didn't only talk about OS X. There is still no splitter visible at least on Windows. Is this the final decision? IMO it makes hard to understand why an empty are inside the location bar could be used to resize only this two elements.

Comment 80

[Dão Gottwald [:dao]]

(In reply to comment #79)
> There is still no splitter visible at least on Windows. Is this the final
> decision?

Nothing is set in stone, but it's the most recent decision and could likely be the final one.

I also support this decision, as it makes the UI less cluttered. The splitter isn't something that you need to see all the time. The cursor makes it discoverable, as Alex already pointed out rightly.

Comment 81

[Γριφεγ]

The splitter is a functional area of the UI. These areas are in my opinion better marked as such.

For example, have you ever visited a website which had links styled with no underline? It looks less cluttered, but very clumsy to use.

I believe this is true in general. If the look on Windows Vista appeared too cluttered or «heavy», that is a flaw of the Windows widget designs or theme.

Now, a toolbar splitter is not a very important UI element, but I am using GNOME with the Fedora Nodoka theme, and splitters already have a very subtle appearance, almost invisible. Still, I much prefer them to be there instead of having an invisible UI element.

And if this is an issue of trying to make Firefox look native, and prominent splitters are part of the native look, such as on NeXT/Cocoa, doesn't it make Firefox feel foreign not to use them?