620963 - Apply persona to about:addons specific chrome

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Garrett]

The Addon Manager no longer shows the address bar when viewed. If you have the old-style menu hidden and don't have the addon bar shown, that means you'll have exactly one line for the tab bar which shows any of the persona. The Addon Manager is where one goes to change personas, yet now you can't really see them there at all. To see what it actually looks like you have to switch to a normal content tab.

The Addon Manager has its own back/forward buttons and search field as well as the update menu button. This is in essence the Addon Manager's navigation toolbar, and as such should have the persona shown behind it just as the normal navigation toolbar and any other toolbars would have. Doing this would give multiple lines of additional space to actually see what the persona you're selecting looks like and would also significantly improve the look of integration between the Addon Manager and the main browser chrome.

Comment 1

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Screenshot: Current persona display problem

Comment 2

[Dave Garrett]

On Linux this issue is worse than on other platforms where we support drawing in the title bar. Linux doesn't support that yet so it only sees the Persona behind the tab bar if enabled, whilst Mac and Windows can get another line's worth if they have the tab bar and the title bar. That's still not that much in comparison to the available space here.

Comment 3

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Mockup: Adding an opacity gradient to the current persona in the add-ons manager

In order to show the current persona in the add-ons manager, a gradient to 0% opacity could be applied to gradually connect the persona with the page.  This isn’t an exact match of how the persona will look elsewhere, but it clearly shows which persona is being selected, maintains its top position to prevent a jarring effect when tab-switching, and is less ugly than cutting the persona off with a hard line.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Taking this, somewhat accidentally (was bored, starting playing around, found out I had a solution: http://grab.by/8Aue).

Comment 5

[[not reading bugmail]]

What about when Jimm works on bug 590945 to do bug 513170 applying personas to the entire window frame?

Comment 6

[[not reading bugmail]]

we really should get rid of that black border on the addon bar tab anyway even if we don't do this to blend better with lwthemes and make the tab grey too to blend up.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #5)
> What about when Jimm works on bug 590945 to do bug 513170 applying personas to
> the entire window frame?

Huh, I hadn't seen those mockups in bug 513170 before. I *think* it may just magically work (and look good), since the effect is achieved via transparency of the addons manager page. Will be interesting to see how it turns out, after 4.0.

(In reply to comment #6)
> we really should get rid of that black border on the addon bar tab anyway even
> if we don't do this to blend better with lwthemes and make the tab grey too to
> blend up.

Yep, that's what I spent a few hours figuring out today (took me *ages* to hunt down). Also added the lighting effect that the toolbars normally have. With those two tweaks, it looks much better: http://grab.by/8C0i

I still need to test with other OS themes - notably Windows classic (dark theme), Windows XP, and a dark Linux theme. In those cases, we use system colors for the background - which you can't just alter the alpha channel. So I'm using a hack at the moment (transparent white) - which I think may not look so nice on a dark theme :\

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #7)
> which I think may not look so nice on a dark theme :\

Turns out it works just fine. Success!

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Note: The change in browser.css is needed to not regress bug 543306.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 508317 [details] [diff] [review]
Patch v1

r=me for the toolkit JS stuff, I'll let dao look over the theme bits.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 508317 [details] [diff] [review]
Patch v1

>--- a/browser/base/content/tabbrowser.css
>+++ b/browser/base/content/tabbrowser.css

> tabpanels {
>+  background-color: transparent;
>+}
>+
>+browser:not([transparent="true"]) {
>   background-color: white;
> }

Does this affect the appearance of notification bars? E.g. while they are fading in?
How about the web console?

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 508317 [details] [diff] [review]
Patch v1

>--- a/browser/themes/gnomestripe/browser/browser.css
>+++ b/browser/themes/gnomestripe/browser/browser.css

>+#main-window[disablechrome]:-moz-lwtheme #navigator-toolbox {
>+  border-bottom: none;
>+}

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+#main-window[disablechrome]:-moz-lwtheme #navigator-toolbox::after {
>+  height: 0px;
>+}

I think the fix for bug 631698 made this unnecessary.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #7)
> http://grab.by/8C0i

This screenshot worries me... It looks like you're getting grayscale instead of sub-pixel anti-aliased text across the whole addons manager. I suspect your patch triggers this. Is this the case?

Comment 14

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Screenshot: text closeup, seemingly without anti-aliasing, from above posted screenshot

(In reply to comment #13)
> (In reply to comment #7)
> > http://grab.by/8C0i
> 
> This screenshot worries me... It looks like you're getting grayscale instead of
> sub-pixel anti-aliased text across the whole addons manager. I suspect your
> patch triggers this. Is this the case?

It does looks like the anti-aliasing is gone from the text, creating stark black lines.  Is that going on, Blair?

Comment 15

[[not reading bugmail]]

It would be nice to see this in Fx4, but I noticed there is no target for this.

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 508317 [details] [diff] [review]
Patch v1

Need responses to the previous comments. Also, I think it's getting too late to take this for Firefox 4.

Comment 17

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Ugh, sorry, I thought I had commented awhile ago. Yes, I'm punting this to post-4.0. 

AFAIK, the anti-aliased text issue is due to making the browser element transparent. Which would need a platform fix (I believe there's a plan for this, though I forget the bug#), or a different strategy where the document is opaque and applies the background image with the right offsets (not sure how we'd work out when those offsets should dynamically update though).

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Talked to roc, looked through a bunch of code, and filed bug 659914 - seems we should be getting subpixel AA now.

Updated and un-bitrotted the patch. Once bug 659914 is fixed (on the assumption that it can be fixed), it'll need additional review from a browser-peer, since I needed to take into account the fix for bug 558585. I'll hold off until then, since it'll probably bitrot again before then.

Comment 20

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.