Set error console toolbar to use browsertabbar-toolbox

VERIFIED FIXED in Firefox 3

Status

()

Firefox
Theme
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: faaborg, Assigned: Away for a while)

Tracking

Trunk
Firefox 3
x86
Windows Vista
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
As a follow up to bug 426744, let's try -moz-appearance: browsertabbar-toolbox for the error console toolbar for Aero.
(Assignee)

Updated

10 years ago
Assignee: nobody → ehsan.akhgari
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

10 years ago
Created attachment 318249 [details] [diff] [review]
Patch (v1)

Simple patch.  I'll attach a screenshot for Alex to ui-review.
(Assignee)

Comment 2

10 years ago
Created attachment 318250 [details]
Screenshot
Attachment #318250 - Flags: ui-review?(faaborg)

Comment 3

10 years ago
Looks rather clumsy to me...
This is also different from bug 426744, where we have buttons (not toolbar buttons) and only one toolbar.
(Reporter)

Comment 4

10 years ago
Comment on attachment 318250 [details]
Screenshot

shiny :)
Attachment #318250 - Flags: ui-review?(faaborg) → ui-review+
(Assignee)

Comment 5

10 years ago
Comment on attachment 318249 [details] [diff] [review]
Patch (v1)

Trivial patch to review.
Attachment #318249 - Flags: review?(gavin.sharp)
(Reporter)

Comment 6

10 years ago
Since we are in the error console, filed follow up bug 431241.
(Assignee)

Updated

10 years ago
Depends on: 431309
(Assignee)

Comment 7

10 years ago
Created attachment 319945 [details] [diff] [review]
Patch (v1.1)

Updated patch according to bug 431309.
Attachment #319945 - Flags: review?(dao)
(Assignee)

Updated

10 years ago
Attachment #318249 - Attachment is obsolete: true
Attachment #318249 - Flags: review?(gavin.sharp)

Comment 8

10 years ago
Comment on attachment 319945 [details] [diff] [review]
Patch (v1.1)

Code-wise this is a stretch since that toolbar isn't a tabbar. Are we sure that we're changing the console window for the better?
(Assignee)

Comment 9

10 years ago
Alex has ui-r+'ed this change.  Should I another ui-r on this?  Alex: what do you think?
That was a pretty quick ui-review, and since Alex proposed the change that's not surprising. We don't strictly need another ui-review, but a second thought would be nice. I'm not saying that the screenshot looks disastrous, but if you put it next to the current window, I'm not sure it's an improvement.
(Assignee)

Comment 11

10 years ago
Comment on attachment 319945 [details] [diff] [review]
Patch (v1.1)

OK, let's see what Beltzner thinks.
Attachment #319945 - Flags: ui-review?(beltzner)
I think Mike is unavailable for the rest of the week.
(Assignee)

Comment 13

10 years ago
(In reply to comment #12)
> I think Mike is unavailable for the rest of the week.

Oh, do you know anyone else to ask about this until Mike returns?
Mike Connor theoretically...
But we could also just wait. I'm not entirely sure how release drivers are going to handle stuff like this, but I guess it could land between RC1 and RC2.
(Reporter)

Comment 15

10 years ago
Just to quickly explain my rationale, every toolbar in an updated Vista application has a centered reflection line (Windows Explorer, Internet Explorer, Media Player, Photo Gallery, Windows Movie Maker, Windows Calendar, Windows Mail). The only instance of a silver toolbar is in IE, but it is just as much a toolbar as it is a tab strip, containing controls for bookmarks, home, rss, print page and tools.

In July I asked a program manager on the windows user experience team:

>How should user experience designers determine the appropriate
>toolbar color when creating an application for Vista?  It would appear:

>green = system (Windows Explorer, Windows Contacts, Open dialog box)
>blue = productivity (Windows Calendar, Windows Mail)
>black = media (Windows Media Player, Windows Movie Maker, Windows
>Photo Gallery)

>Is this distinction correct?
>-Alex

The response I received was:

>Good question! Your distinctions are correct, but our recommendation
>is silver = neural (IE).

So even though the implementation name indicates that it is used for the tabstrip in IE, the user experience team over there is considering it the recommended appearance for tool bars.

However we style the error console isn't a big deal, but in general I think we need to be careful to make a distinction between platform native in terms of what we are able to extract, and platform native in terms of what the platform actually looks like.  For instance, toolbox and InfoBackground are still available for legacy support, but they simply don't appear anywhere in an updated Vista application running with the Aero theme.
Comment on attachment 319945 [details] [diff] [review]
Patch (v1.1)

Thanks for the explanation, Alex. I still don't particularly like the result, but at least now I know that there's a rationale behind it.

>+#ToolbarMode  {
>+  -moz-appearance: -moz-win-browsertabbar-toolbox;
>+}
>+
>+#ToolbarEval  {
>+  -moz-appearance: toolbox;
>+}

nit: superfluous space before the opening brackets.
Attachment #319945 - Flags: ui-review?(beltzner)
Attachment #319945 - Flags: review?(dao)
Attachment #319945 - Flags: review+
(Assignee)

Comment 17

10 years ago
Created attachment 320127 [details] [diff] [review]
Patch (v1.2)

Addressed Dão's nit in comment 16.  Requesting approval for this CSS-only change which attempts to match the Vista style toolbars in the Error Console window.
Attachment #319945 - Attachment is obsolete: true
Attachment #320127 - Flags: approval1.9?

Comment 18

10 years ago
Comment on attachment 320127 [details] [diff] [review]
Patch (v1.2)

a+ schrep  please test this on tomorrow's nighties because we have zero time for respins.
Attachment #320127 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
mozilla/toolkit/themes/winstripe/global/jar.mn 	1.60
mozilla/toolkit/themes/winstripe/global/console/console-aero.css 	1.1 
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008051506 Minefield/3.0pre ID:2008051506
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.