866354 - Social notification icons are too wide and not jutting right up next to each other

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of bug

See attached image.

The badge area is taking up some visible width even when the badge is not visible. The notification buttons shouldn't have any margin between them either. 

We'll have to do something like position the badge absolutely to remove it from expanding the width of the button, and probably double-check the left/right margin rules on the buttons.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: fix windows button width

we need to override a default style from toolkit css.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 743377 [details] [diff] [review]
fix windows button width

Review of attachment 743377 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, but there is still a gap between the notification icons.

browser/themes/windows/browser.css defines:
> @navbarLargeIcons@ .toolbarbutton-1:not([type=menu-button]) {
>   padding:5px 2px;
>   ...
> }

This horizontal padding is what creates the gap between the buttons. We probably need to add the following rules:

> @navbarLargeIcons@ #social-toolbar-item {
>   margin-left: 2px;
>   margin-right: 2px;
> }
> 
> @navbarLargeIcons@ #social-toolbar-item > .toolbarbutton-1 {
>   padding-left: 0;
>   padding-right: 0;
> }

This should fix the gaps. There is another issue in that we are no longer drawing the separators between the buttons. http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#835 doesn't apply to anything anymore since the .social-notification-container class isn't used anymore. The .social-notification-container references can probably be replaced with .toolbarbutton-1 references.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: fix windows button width

updated with feedback, manually tested on win7

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 743791 [details] [diff] [review]
fix windows button width

Review of attachment 743791 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks for fixing this!

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Also happening on Aurora.

Comment 6

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4a171373526

Comment 7

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 743791 [details] [diff] [review]
fix windows button width

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 855389
User impact if declined: social buttons are extra wide, take a lot of space
Testing completed (on m-c, etc.): manually on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a4a171373526

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7fe1490b0778

Comment 10

[Manuela Muntean [Away]]

Verified fixed with Firefox 22 beta 1 (build ID: 20130514181517) on Windows 7 64bit.

Comment 11

[u279076]

Samvedana, can you please verify this is fixed in Firefox 23?

Comment 12

[Samvedana (:Samvedana)]

Attachment: icons in FX 23 B4

User Agent : Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20130430 Firefox/23.0
Build ID : 20130430030941

Tested on nightly and I am able to reproduce this issue on this build.

User Agent : Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Build ID : 20130708202947

Tested on Firefox 23-B4, I can see desired result. Please see the attached file.