New toolbarbutton style for the main window

RESOLVED FIXED in Firefox 3.7a2

Status

()

enhancement
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Depends on 1 bug)

Trunk
Firefox 3.7a2
All
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Assignee

Description

9 years ago
Posted patch patch (obsolete) — Splinter Review
No description provided.
Attachment #425871 - Flags: review?(rflint)
Assignee

Comment 1

9 years ago
Posted patch patch v2 (obsolete) — Splinter Review
Fixed the rtl forward button... I had to flip the whole button rather than the icon, otherwise I would have needed a second svg mask for rtl.
Attachment #425871 - Attachment is obsolete: true
Attachment #425938 - Flags: review?(rflint)
Attachment #425871 - Flags: review?(rflint)
Assignee

Comment 2

9 years ago
Posted patch patch v2 (obsolete) — Splinter Review
I screwed up copying and pasting...
Attachment #425938 - Attachment is obsolete: true
Attachment #425939 - Flags: review?(rflint)
Attachment #425938 - Flags: review?(rflint)
Some notes:
 - The back button isn't a circle; it's 32px high and 30px wide.
 - I think the disabled button state is too dark.
 - The fact that hovering the buttons is animated is not noticeable at all
   because the icon itself changes immediately.
 - The click animation is inconsistent because it doesn't happen when the
   button becomes disabled afterward.
 - Checked buttons don't have a mousedown state.
Assignee

Comment 4

9 years ago
(In reply to comment #3)
> Some notes:
>  - The back button isn't a circle; it's 32px high and 30px wide.

Interesting, I guess some margin is messing this up.

>  - I think the disabled button state is too dark.

I'll test something different, but I'm not going to spend too much time on it, as we can easily and should tweak this once it has landed.

>  - The fact that hovering the buttons is animated is not noticeable at all
>    because the icon itself changes immediately.

The final icons probably won't have hover states (already the case for 7/Vista).

>  - The click animation is inconsistent because it doesn't happen when the
>    button becomes disabled afterward.

I'll further play around with this as well.

>  - Checked buttons don't have a mousedown state.

This is intentional, but I'll double check with a native app if I can find one.
Assignee

Comment 5

9 years ago
(In reply to comment #4)
> >  - Checked buttons don't have a mousedown state.
> 
> This is intentional, but I'll double check with a native app if I can find one.

Explorer applies the normal hover state to checked buttons, which looks awkward.
Assignee

Comment 6

9 years ago
Posted patch patch v3Splinter Review
various tweaks
Attachment #425939 - Attachment is obsolete: true
Attachment #425972 - Flags: review?(rflint)
Attachment #425939 - Flags: review?(rflint)
Comment on attachment 425972 [details] [diff] [review]
patch v3

>diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
+    <svg:mask id="winstripe-keyhole-forward-mask" maskUnits="objectBoundingBox" maskContentUnits="objectBoundingBox">
>+      <svg:rect x="0" y="0" width="1" height="1" fill="white"/>
>+      <svg:circle cx="-0.42" cy="0.5" r="0.64" id="circle" fill="black"/>

objectBoundingBox and black are the default values for maskUnits and fill, so they can be omitted.

Looks good, r=me. The only issue I can see is that the back button padding no longer extends to the window border per Fitts.
Attachment #425972 - Flags: review?(rflint) → review+
Assignee

Comment 8

9 years ago
Posted patch patch v4Splinter Review
addressed the review comment and made some more adjustments after discovering https://wiki.mozilla.org/images/8/82/Firefox-4-Mockup-i05-%28Win7%29-%28Aero%29-%28TabsTop%29-%28ButtonStates%29.png
Assignee

Comment 9

9 years ago
(In reply to comment #8)
> Created an attachment (id=426532) [details]
> patch v4
> 
> addressed the review comment

... the one about the attribute defaults. I didn't make the back button extend to the window border, as I currently don't know how to do it.
(In reply to comment #8)
> Created an attachment (id=426532) [details]
> patch v4
> 
> addressed the review comment and made some more adjustments after discovering
> https://wiki.mozilla.org/images/8/82/Firefox-4-Mockup-i05-%28Win7%29-%28Aero%29-%28TabsTop%29-%28ButtonStates%29.png

That's totally my fault. All the current mockups are here: https://wiki.mozilla.org/Firefox/4.0_Windows_Theme_Mockups

Is there a better way to expose the design work? I thought keeping it one place would be good, but I could attach it to bugs if that would be better.
Assignee

Comment 11

9 years ago
I think I prefer keeping it on a single page as well.
Assignee

Comment 12

9 years ago
http://hg.mozilla.org/mozilla-central/rev/247b6aed5414
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a2
Assignee

Updated

9 years ago
Depends on: 545842
Assignee

Updated

9 years ago
Depends on: 546261
Assignee

Updated

9 years ago
Depends on: 546262
Maybe I missed something, but the arrows aren't supposed to have a tail (like the mockups) ?

Updated

9 years ago
Depends on: 546774

Comment 14

9 years ago
I believe this check-in is responsible for the main menu lagging after some time.  The longer the browser has been up, the longer it takes a menu to appear when clicked.

It there any way to turn off that toolbar button animation?  The fact that it gets worse over time probably means there's a leak somewhere, but I'm speculating.

Updated

9 years ago
Depends on: 547426

Comment 15

9 years ago
Looking at the mock-ups and the "real deal", I see two major differences. The back arrow is at least one pixel too far to the right, which, while it would mean it isn't completely centered, actually looks better, in my mind. The other issue I can see is that they don't seem to have the glow/brightness nor shadow that the mock-ups show. These differences do, in my opinion, "degrade" the quality of the theme severely.
Assignee

Updated

9 years ago
Depends on: 547752

Comment 16

9 years ago
(In reply to comment #14)
> I believe this check-in is responsible for the main menu lagging after some
> time.  The longer the browser has been up, the longer it takes a menu to appear
> when clicked.

There does seem to be a problem affecting both the main menu and the bookmark toolbar. It worsens with time but disappears after a restart.

The stop button seems a bit too large in the small icons mode when compared to the back/forward buttons. Also, it would be better if Firefox does not interfere with 3rd party button theming. The new square button style on the 3rd party buttons makes it look uglier.

Comment 17

9 years ago
- The icons are now monochrome, thus losing the advantage of color recognition for quickly targeting an icon (color is more important than shape, IIRC). 

- Windows 7 icons (e.g. Windows Explorer & Internet Explorer) are color icons. Why are we being inconsistent with the OS (and for the worse)?

- The icon images seem the same size in normal and in small icon sizes. That loses a big part of the advantage of having large icons. Are we ignoring people with non-good eyesight?

If this bug (about "style") is not where this change to monochrome was made, which bug is it? Is it bug 546098? Where is a good place to input these objections?
Assignee

Comment 18

9 years ago
(In reply to comment #17)
> Where is a good place to input these objections?

mozilla.dev.apps.firefox
Assignee

Updated

9 years ago
Depends on: 548027
Assignee

Updated

9 years ago
Depends on: 547440
Assignee

Updated

9 years ago
Depends on: 429062
Assignee

Updated

9 years ago
Blocks: 431263
Assignee

Updated

9 years ago
Blocks: 439822
Assignee

Updated

9 years ago
Depends on: 553673
Assignee

Updated

9 years ago
Depends on: 583285
You need to log in before you can comment on or make changes to this bug.