462644 - Remove etch from the Keyhole for Firelight

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

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

Currently one of the most common complaints about our OS X theme is the etch we created to try to group the back and forward controls to the drop down arrow.  It makes the controls look a little more complicated, and this type of navigation drop down isn't found elsewhere in the OS.  Also, the etch seems to provide a visual affordance that you can slide the forward button left and right, which wasn't helped by the release of the iPhone (where you actually have this kind of control).

The cleanest way to solve this problem is to completely remove both the etch and the drop down.  Overall I think this is the best idea, and (if I remember correctly) beltzner agrees with removing the control on OS X since this is more platform native.  Beltzner: can you comment to make sure this is the case.

Alternatively I guess we could make the etch much lighter, and less of a hard line and more of a gradual push back into the surface.  We can modify the bug to go with this solution if beltzner doesn't want the drop down control removed.

Regardless of if we keep the etch or not, I think we should remove the left side, since the curve intersects the back button a little strangely.  We are planning on removing that side of the etch on windows as well.

Comment 1

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

comments from beltzner sent in email:

>I'm more ok with it on OSX than w32, yes, though I remain hesitant. I think 
>we need to present reasons for doing so, as we're basically trading pixel 
>space for a far less discoverable ui (the only other OSX app that behaves 
>that way is Safari) and the question of why that bit as opposed to just 
>making the keyhole smaller will definitely come up. It's probably ok, 
>though.

Comment 2

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

My reply:

>the question of why that bit as opposed to just
>making the keyhole smaller will definitely come up.

It isn't about reducing size but visual complexity, there is a whole lot going on with the etch and the drop down right now.

Arguments in favor of the change:

I think users will be able to discover right click (if they are windows people) or click and hold, if they really want to find if the functionality is still there.  If someone just switched to a Mac the absence of a drop down arrow in Firefox isn't going to be the most jarring part of the overall transition to the new OS.

I think we should drop the control for OS X based on the "when in rome" argument, although traditionally we have tried hard to have a consistent interactive design across platforms.

Comment 3

[Shawn Wilsher :sdwilsh]

I'll be happy to take this once we get a new image to drop in.  I'll also have to adjust the CSS of the other images that come after the back/forward buttons if the width ends up changing.  Knowing what the dimensions of each region is a big help there (I have no good tools to determine that information).

Comment 4

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

Stephen has the source material so he will need to attach the new toolbar.png

Comment 5

[Stephen Horlander [:shorlander]]

Attachment: Toolbar.png without etch

Here is a new Toolbar.png without the etch.

Comment 6

[Shawn Wilsher :sdwilsh]

I'll also need a new rtl image for this.

Comment 7

[Shawn Wilsher :sdwilsh]

Attachment: v0.1

Need the rtl image to finish this bug, but this is the ltr stuff.

Comment 8

[Stephen Horlander [:shorlander]]

Attachment: Toolbar-rtl.png without etch.

Here is the RTL version. Sorry about that.

Comment 9

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

Follow up bug 465667 with several alignment tweaks.

Comment 10

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

I think this get's rtl.  I ended up changing the chromedir manually on the buttons and switched their order.  Nobody was around on irc that could tell me how to actually change the entire UI to use rtl for the whole UI to make sure it looks right.

Once I find that out, I'll know for sure.

Comment 11

[Markus Stange [:mstange]]

Use https://addons.mozilla.org/en-US/firefox/addon/7438 :-)

Comment 12

[Shawn Wilsher :sdwilsh]

Attachment: screenshot (ltr)

Comment 13

[Shawn Wilsher :sdwilsh]

Attachment: screenshot (rtl)

Comment 14

[Shawn Wilsher :sdwilsh]

Comment on attachment 349018 [details] [diff] [review]
v1.0

forgot about small icons in rtl...

Comment 15

[Shawn Wilsher :sdwilsh]

Attachment: screenshot small icons (ltr)

Comment 16

[Shawn Wilsher :sdwilsh]

Attachment: screenshot small icons (rtl)

Comment 17

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

Comment 18

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

minor change so I don't have to change one of the selector rules that is part of the default buttons (possible perf hit).

Comment 19

[Dão Gottwald [:dao]]

Comment on attachment 349094 [details] [diff] [review]
v1.2

> toolbar[mode="icons"] #unified-back-forward-button > #forward-button[disabled="true"] {
>- -moz-image-region: rect(33px, 566px, 66px, 539px);
>+ -moz-image-region: rect(33px, 561px, 66px, 536px);
> }
> 
> toolbar[mode="icons"] #unified-back-forward-button > #forward-button[disabled="true"][chromedir="rtl"] {
>- -moz-image-region: rect(33px, 547px, 66px, 519px);
>+ -moz-image-region: rect(33px, 529px, 66px, 504px);

While you're at it, you could fix the indention...

Comment 20

[Shawn Wilsher :sdwilsh]

(In reply to comment #19)
> While you're at it, you could fix the indention...
Fixed in a few other places that I'm touching, and I added vim and emacs modelines to the file.

Comment 21

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

Since this bug is pending ui-r I wanted to jump in and provide some quick design rationale.  There are two design attributes in contention here: creating a discoverable and easy to use interface, vs. creating an interface that is visually clean.

So I believe that it isn't the case that one approach is right and the other wrong, it's just a choice of how we want to balance discoverability of features with visual design.  Also, I don't think that user's necessarily value discoverability so much more than visual design that this is a major mistake, or the other way around.  Both solutions are balanced tradeoffs.

However, I think by not being willing to adjust this balance to the balance established by each platform, we end up being a foreign application (even if we got all the colors and gradients perfect).  Asserting that a particular balance between discoverabilty and visual design is universally correct for all platforms seems to disregard that platforms have expected norms, and users of those platforms have established values.  In particular I believe that one of the things preventing us from taking more market share away from Safari on OS X is the perception that Firefox is "easy to use, but messy" when users actually want "harder to use, but clean."  They aren't arguing against the fact that seeing the drop down makes it easy to discover, they are asserting that you only have to discover the interaction once, but you have to actually look at the thing thousands of times, and they expect us to take a different balance.

Comment 22

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

Alex - thanks for the context. Based on your rationale, taking this bug independent of other bugs that work to overhaul the look of default chrome might leave us in a neither-this-or-that state in terms of choosing discoverability over visual cleanliness. Is that right?

Two additional questions come to mind:

 - is this the first time we would present different default chrome based on the operating system? aside from icons, we've been trying to create a XPP Firefox look using shape, haven't we? wouldn't this change that?

 - click-and-hold on the back button would now potentially show entries that would take the user forward; if we implement this change, should we not ensure that the lists are separated out again?

Comment 23

[Shawn Wilsher :sdwilsh]

(In reply to comment #22)
>  - is this the first time we would present different default chrome based on
> the operating system? aside from icons, we've been trying to create a XPP
> Firefox look using shape, haven't we? wouldn't this change that?
We still have the keyhole design, it just get's rid of the non-osxy groove (for lack of a better word) that they sit in.  Based on shape, this changes nothing.

>  - click-and-hold on the back button would now potentially show entries that
> would take the user forward; if we implement this change, should we not ensure
> that the lists are separated out again?
We already do this now, so I'm not really sure how this is an argument against taking this...

Comment 24

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

>Alex - thanks for the context. Based on your rationale, taking this bug
>independent of other bugs that work to overhaul the look of default chrome
>might leave us in a neither-this-or-that state in terms of choosing
>discoverability over visual cleanliness. Is that right?

There are a range of options from easier to use to visual cleanliness:

-current UI [disoverablity]
-much softer etch
-much softer etch,just one side
-drop down floating in space next to the keyhole
-no drop down [visual cleanliness]

> - is this the first time we would present different default chrome based on
>the operating system? 

The current changes between all platforms are of course things like action button placement on dialogs, "Preferences" vs. "Options," and using native print and open dialogs.  But I think as precedent I would probably cite not using the keyhole at all on Linux.  Just as GTK icons were the native choice for that platform, trying to push the bounds of visual cleanliness is the native choice for OS X.

>aside from icons, we've been trying to create a XPP
>Firefox look using shape, haven't we? wouldn't this change that?

They keyhole is iconic enough by itself, in fact deemphasizing the etch and drop down probably makes it more iconic.

> - click-and-hold on the back button would now potentially show entries that
>would take the user forward; if we implement this change, should we not ensure
>that the lists are separated out again?

I think ideally we would depress both buttons and show the integrated list when you pressed and held either.  Bringing back the separate lists would solve the general UI problem and would still be better than an integrated list without depressing both buttons.

Comment 25

[Shawn Wilsher :sdwilsh]

(In reply to comment #24)
> I think ideally we would depress both buttons and show the integrated list when
> you pressed and held either.  Bringing back the separate lists would solve the
> general UI problem and would still be better than an integrated list without
> depressing both buttons.
We'd need an additional image with both buttons depressed for this, as well as modifying the controls of the buttons (all doable, but perhaps follow-up?).

Comment 26

[Shawn Wilsher :sdwilsh]

Someone else can drive this in if ui-r feedback is negative, but I've got to spend time on other stuff.

Comment 27

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

We've decided to keep the drop down arrow so we aren't doing an interactive chance, but remove the etch to streamline the appearance.  I'll post the new toolbar.png files.

Comment 28

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

Attachment: Toolbar without keyhole etch (but with drop down)

Comment 29

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

Attachment: Toolbar-rtl without keyhole etch (but with drop down)

Comment 30

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

If anyone is able to pick this up very quickly so that we can get it into 3.5, please do.  Also note a few alignment tweaks from the current patch detailed in bug 465667.

Comment 31

[Markus Stange [:mstange]]

Uh... are the color profiles attacking again? The new images look darker than the current Toolbar.png. And the current Toolbar-rtl.png is too dark, too...

Comment 32

[Markus Stange [:mstange]]

Attachment: image region changes

These are the CSS image region changes. My build is still compiling, but I have to leave now, so this is basically untested... I won't be able to work on this for the next 9 hours, so whoever can, please take over.

Comment 33

[Dão Gottwald [:dao]]

(In reply to comment #31)
> Uh... are the color profiles attacking again? The new images look darker than
> the current Toolbar.png. And the current Toolbar-rtl.png is too dark, too...

Indeed. The gAMA chunk is missing, used to be 0.55000.

Comment 34

[Dão Gottwald [:dao]]

Attachment: Toolbar.png (gAMA added)

Comment 35

[Dão Gottwald [:dao]]

Attachment: Toolbar-rtl.png (gAMA added)

The old Toolbar-rtl.png didn't have a gAMA chunk, but it needs one to be consistent with Toolbar.png.

Comment 36

[Dão Gottwald [:dao]]

Attachment: patch

Comment 37

[Dão Gottwald [:dao]]

Attachment: patch v2

forgot about the click-and-hold behavior on OS X...

Comment 38

[Markus Stange [:mstange]]

Comment on attachment 379537 [details] [diff] [review]
patch v2

Looks like you found all my mistakes.

Comment 39

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/32029d575146

Comment 40

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

Comment on attachment 379537 [details] [diff] [review]
patch v2

a191=beltzner; this change is going to shock a bunch of nightly users, so you might want to make sure that there's a blog entry that goes along with it so we don't get a lot of "OMG BUG!" reports.

Comment 41

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

>this change is going to shock a bunch of nightly users, so you
>might want to make sure that there's a blog entry that goes along with it so we
>don't get a lot of "OMG BUG!" reports.

sure thing, I'll watch this bug to get the post up before the nightly it lands in.

Comment 42

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d7cbcbc4b7e9

Comment 43

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

This breaks Personas, at least, pretty badly:

http://www.grabup.com/uploads/cb3a86731ec9571b3bcdf780c85314dd.png

Which makes me suspect that it may also affect other themes.

It's a little-to-a-lot late to be hitting theme compatibility. Did we give the Personas guys a heads-up? If we suspect that this will affect compatibility in the slightest, then we need to back it out and wait until next time.

Re-opening and blocking until those questions are answered.

Comment 44

[Dão Gottwald [:dao]]

Nope, this can't possibly affect other themes. But personas is an extension that hacks the theme, so yes, it's no surprise that personas is affected. Should be pretty easy for them to update their custom keyhole images, though.

Comment 45

[Dão Gottwald [:dao]]

... or if it's somehow hard or undesirable to update the image, they could keep the etch and explicitly use the old coordinates for their old Toolbar.png.

Comment 46

[Aaron Train [:aaronmt]]

On OSX with today's (20090926) shiretoko build and the removal of the etch overnight

http://i43.tinypic.com/2psn53r.png

it reverts back with a personas theme as in comment #43 to look like 
http://www.grabup.com/uploads/cb3a86731ec9571b3bcdf780c85314dd.png

Comment 47

[Aaron Train [:aaronmt]]

^20090526

Comment 48

[Myk Melez [:myk] [@mykmelez]]

As Dão notes, Personas is particularly aggressive with toolbarbuttons on Mac OS X, providing its own versions of all the standard buttons in order to set their transparency and antialiasing appropriately so they look good when a persona is selected.

Thus the regression here is likely to be fairly specific to Personas rather than a general regression that affects many extensions, and the onus should probably be on Personas to update its custom toolbarbutton images to reflect the change in the default Firefox theme.

I have filed bug 494925 on fixing this problem in Personas.  I think no additional work is needed in this bug, and it can be resolved fixed again.

(FWIW, those transparency/antialiasing fixes should probably get pushed into core so Personas doesn't have to ship its own versions of the icons, but that's a separate bug; filed that one as bug 494927).

Dão: what's the problem with this bug blocking bug 494925?  That bug does depend on this one, since it makes no sense to change the Personas images if the Firefox images don't change.

Comment 49

[Dão Gottwald [:dao]]

(In reply to comment #48)
> Dão: what's the problem with this bug blocking bug 494925?

Mid-air? I haven't touched the dependency fields intentionally.

Comment 50

[Marcia Knous [:marcia]]

Based on Comment 48 resolving fixed.

Comment 51

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #49)
> Mid-air? I haven't touched the dependency fields intentionally.

Ah, sorry, that must've been it!


(In reply to comment #50)
> Based on Comment 48 resolving fixed.

Erm, this is still open; resolving.

Note: I've pushed the fix for Personas, so you can confirm that Firefox works fine with the fixed version of Personas by testing this Personas development build:

https://people.mozilla.com/~cbeard/personas/dist/personas-dev.xpi

Comment 52

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

Verified fixed on trunk and 1.9.1 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre ID:20090527031500

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre ID:20090527031214

Comment 53

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

Comment on attachment 379537 [details] [diff] [review]
patch v2

a191=beltzner post-facto and post-fixo

Comment 54

[Dão Gottwald [:dao]]

Approval was already given in comment 40, fwiw.