980115 - Social API status buttons make many other toolbar-buttons vertically squished

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Visit https://talkilla.mozillalabs.com/
 2. Click "Activate Talkilla"
 3. [optional] Enter "Customize" mode, and experiment putting the talkilla icon on the toolbar vs. in the hamburger menu.

ACTUAL RESULTS:
When Talkilla icon goes on the toolbar, most of the toolbar gets vertically squashed. In particular, these are squished:
 - The back button
 - The entire URL bar (and the lock icon)
 - The search bar (its contents nearly overflow, outside of the search box)
 - The download "downarrow" icon
 - The home icon

EXPECTED RESULTS:
 Nothing should be squished.

Comment 1

[Daniel Holbert [:dholbert]]

(I'm going to guess that this is Australis-related, but I'm not sure)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of browser after clicking "Activate Talkilla" [note the squashed toolbar icons]

Comment 3

[:Gijs (he/him)]

(In reply to Daniel Holbert [:dholbert] from comment #1)
> (I'm going to guess that this is Australis-related, but I'm not sure)

what version of Firefox are you using?

Comment 4

[Daniel Holbert [:dholbert]]

Sorry -- up-to-date nightly. 30.0a1 (2014-03-05)

Comment 5

[:Gijs (he/him)]

I think the social API buttons have a max-height on their icon, and that's what's triggering this. Shane, can you look at this? I'm pretty swamped right now. See bug 967115 for more context.

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: fix status button sizing

fixes button sizing in toolbar, menu panel, and customization.  tested osx/win7/linux.

Comment 7

[:Gijs (he/him)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #6)
> Created attachment 8387997 [details] [diff] [review]
> fix status button sizing
> 
> fixes button sizing in toolbar, menu panel, and customization.  tested
> osx/win7/linux.

How do I test the badge container?

Comment 8

[Shane Caraveo (:mixedpuppy)]

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #6)
> > Created attachment 8387997 [details] [diff] [review]
> > fix status button sizing
> > 
> > fixes button sizing in toolbar, menu panel, and customization.  tested
> > osx/win7/linux.
> 
> How do I test the badge container?

install the "demo social provider" from http://mixedpuppy.github.io/socialapi-directory/ and click the "login" button in its sidebar.  the status button will get a badge.  You can then move the button around to different area's to see how it looks.  The icon used by the demo is only 16px so it will look pixilated in the menu panel, but otherwise it should correct.

Comment 9

[:Gijs (he/him)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #8)
> (In reply to :Gijs Kruitbosch from comment #7)
> > (In reply to Shane Caraveo (:mixedpuppy) from comment #6)
> > > Created attachment 8387997 [details] [diff] [review]
> > > fix status button sizing
> > > 
> > > fixes button sizing in toolbar, menu panel, and customization.  tested
> > > osx/win7/linux.
> > 
> > How do I test the badge container?
> 
> install the "demo social provider" from
> http://mixedpuppy.github.io/socialapi-directory/ and click the "login"
> button in its sidebar.

This doesn't do anything for me and instead shows:

JavaScript error: http://mixedpuppy.github.io/socialapi-demo/sidebar.js, line 9: JSON.parse: unexpected character at line 1 column 1 of the JSON data


in the console.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8387997 [details] [diff] [review]
fix status button sizing

This doesn't look obviously wrong code-wise, but I'm uncomfortable giving this r+ without being able to test the badging, which readds some max-height bits (albeit restricted to the panel)

Comment 11

[:Gijs (he/him)]

Comment on attachment 8387997 [details] [diff] [review]
fix status button sizing

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

Well, the good news is it works now that I can test it. I like things that work. :-)

PS: please be mindful of 29 and ask for approval for uplift once this has hit m-c.

Comment 12

[Shane Caraveo (:mixedpuppy)]

Attachment: fix status button sizing V2

This fixes the button icon in the menu panel without max-height, primary fix is removing flex (seems to work elsewhere), but i've also fixed the label wrapping.  We can stay with previous patch or (my preference) use this one.

Comment 13

[:Gijs (he/him)]

Comment on attachment 8388855 [details] [diff] [review]
fix status button sizing V2

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

Sweet! :-)

Comment 14

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8387997 [details] [diff] [review]
fix status button sizing

obsoleted by V2

Comment 15

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/fx-team/rev/86aebfb124f6

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/86aebfb124f6

Comment 17

[:Gijs (he/him)]

Shane, can you ask for uplift on this one? Thanks! :-)

Comment 18

[:Gijs (he/him)]

Totally meant to needinfo for comment #17... :-(

Comment 19

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8388855 [details] [diff] [review]
fix status button sizing V2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): social api status buttons
User impact if declined: toolbar buttons appear squished
Testing completed (on m-c, etc.): fx-team and m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 20

[:Gijs (he/him)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7636911c7ed1

Comment 21

[Francesca Ciceri [:madamezou]]

[bugday-20140416]

Hi,

I was able to reproduce the issue in Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140305030201 CSet: e5b09585215f.
I can confirm that this is now fixed in:
- latest nightly: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140415030203 CSet: 5b6e82e7bbbf

- latest aurora: Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140415004003 CSet: f14047fa8d63

-latest beta:  Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140414143035 CSet: 3a4f085a6398

I'm not able to change the global status to Verified, so I'll just change the flags for the relevant versions.

Cheers,
Francesca

Comment 22

[Daniel Holbert [:dholbert]]

Thanks!