971246 - Fix styling of type="menu" buttons in the menu panel

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[u498184]

Attachment: australis noscript.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140211004037

Steps to reproduce:

1) Install https-everywhere, Stylish or NoScript on a clean profile
2) Add icon to panel


Actual results:

HTTPS Everywhere and Stylish will only allow one other icon on its row: They both knock the third to a new row.  If placed on the menu together, the text for Stylish is cut off.

NoScript's icons will allow you to place 4 icons on a panel rather than the normal 3.  Additionally, the icons are cut off.


Expected results:

Each icon should be spaced evenly throughout the panel

This didn't happen when Australis was originally pushed to Aurora users.

Comment 1

[u498184]

Attachment: australis stylish and https.png

Comment 2

[:Gijs (he/him)]

(In reply to nothingfinerthanscottsteiner from comment #0)
> HTTPS Everywhere and Stylish will only allow one other icon on its row: They
> both knock the third to a new row.  If placed on the menu together, the text
> for Stylish is cut off.

Unfortunately HTTPS Everywhere has its own custom toolbar button binding. I'm not sure if we can actually fix the problem for them.

Stylish just sets orient="horizontal", which breaks all the things. Removing that and forcing a reflow (like by moving it within the panel) makes it work a lot better.

The only thing I'm not sure I understand is how these weren't broken before. Could you try to figure out when this regressed, ie which was the last aurora build on which this worked?

Comment 3

[u498184]

I am unable to install an older build of Aurora, but I updated Aurora to the 29.x branch on Feb 8th.  So it's been one of the updates from the last few days.

Comment 4

[Alice0775 White]

About Stylish toolbutton:
Regressed by
ea246bfdd262	Jared Wein — Bug 897496 - [Australis] Fade out and cut off third-to-nth line of toolbarbutton labels in menupane. r=Gijs ui-r=mmaslaney

Comment 5

[Justin Dolske [:Dolske]]

P2 due to the addons mentioned here all being quite popular. If unregressing 897496 doesn't fix these, we should also consider explicit outreach to have these specific addons fix their code.

Comment 6

[Charles Evans]

In trunk:
Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140215030204 CSet: b80f7eece913
 Noscript icons have become a sliver, both in customize pane and if put into menu panel.
older aurora ff29a2 is FINE: 
full icons show; also Icon labels in menu take up 3 lines, and overlap the next row. 
Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140206004003 CSet: 8563809a50e6

Comment 7

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

Removing keyword since regression was noted in comment #4. Thanks Alice.

Comment 8

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

Updating the summary based on comment #2 and the wontfix'ing of bug 941045.

Comment 9

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Updating the summary based on comment #2 and the wontfix'ing of bug 941045.

Well, I assumed these were related, but it's odd that Alice could pinpoint a regression bug for the behaviour of Stylish in that case - how come that worked before? I expect it was the max-width on the button, and I believe we can put that back without breaking too many things.

Comment 10

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

Attachment: Patch to fix type=menu buttons

I've emailed with the creator of NoScript and described what needs to be changed to fix the NoScript buttons in the panel and palette (removing the max-width:48px). The author has said a fix for them will be coming soon.

This patch fixes the display of type="menu" buttons, which is what the Stylish button is. Except that the Stylish button also has the orient="horizontal" attribute set, which until it is removed will still continue to display the button incorrectly. The button still sits a few pixels lower than the other buttons, but I haven't figured out why that is the case yet.

Note that I couldn't use the same approach for moving the dropmarker as we did for type="menu-button" since the dropmarker for type="menu" is not a sibling to the button, but a child of the button.

Comment 11

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

Attachment: stylish-pre897496.png

This is what the Stylish add-on looked like prior to bug 897496 landing. It wasn't perfect, but didn't look horrible either.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8383903 [details] [diff] [review]
Patch to fix type=menu buttons

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

Yeah, not sure what's causing that little misalignment there. *sigh*, layout funk. We might want to ask somebody in #layout? dholbert maybe?

Anyhow, I think this is better than what we're currently doing, so let's take it. I've updated the Australis add-on compat wiki page talking about avoiding orient="horizontal". As Gijs pointed out in https://bugzilla.mozilla.org/show_bug.cgi?id=941045#c29, usage isn't too widespread it seems, so we're probably fine for that.

Have we reached out to the Stylish author to tell them about removing the attribute? It's a popular add-on, and we shouldn't break it.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

needinfo'ing about getting in touch with the Stylish author(s).

Comment 15

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

(In reply to Mike Conley (:mconley) from comment #14)
> needinfo'ing about getting in touch with the Stylish author(s).

Not yet, I wanted to land this and then get in touch so they don't have a moving target.

Comment 16

[:Gijs (he/him)]

Clearing needinfo per comment #15, updating summary.

Comment 18

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

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Carlos [nohamelin]]

Attachment: A button of type="menu" (Language), after patch, Linux

In Linux Nightly, no dropmarker is visible with this patch.

Comment 21

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

Thanks for commenting, yeah, chrome://browser/skin/toolbarbutton-dropdown-arrow.png doesn't exist on Linux. We'll need to do something different here for Linux. Can you please file a bug for this and mark that bug as blocking this bug?

Comment 22

[u428464]

The add-on buttons seem also to be taller than the (already big) classic buttons in the menu panel.

Comment 23

[Carlos [nohamelin]]

Ok, it's bug 979610. The misalignment of the button needs be covered by other bug.

This bug will be backported to Aurora?

Comment 24

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

If it is fixed in time to backport to Aurora, then yes :)

Comment 25

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

Comment on attachment 8383903 [details] [diff] [review]
Patch to fix type=menu buttons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): issue since beginning of australis
User impact if declined: poor display of type=menu buttons in the panel
Testing completed (on m-c, etc.): locally and on m-c
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

There is a follow-up bug that needs to be fixed. That bug is marked as blocking this bug. The follow-up bug is about the dropmarker icon not appearing on Linux. It should be simple to fix that issue, but I don't want to hold back this fix from Aurora in the meantime.

Comment 26

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d40e9c054bc3

Comment 27

[Ioana (away)]

I tried to reproduce this issue several times on Nightlies from February 5th and 11th:
Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0

It didn't reproduce at all with any of the add-ons listed in this bug.

Reporter, could you please verify that this bug is fixed for you on Firefox 29 and 30?

Comment 28

[u498184]

• Noscript now displays properly
• Stylish's label no longer gets cut off, but it's still not in the correct position
• HTTPS Everywhere still pushes the third icon to a new row and no label is displayed

Tested in both Aurora (30.0a2, 2014-04-07) and Nightly UX (31.0a2, 2014-04-07)

Comment 29

[u498184]

Attachment: australis menu bug.png

Comment 30

[u498184]

Reopened because it isn't resolved

Comment 31

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

Thanks for the feedback. Can you please file a new bug that blocks this bug? The patch in this bug may not be the one that broke this, and tracking multiple patch landings in one bug gets very confusing.

Comment 32

[Ioana (away)]

Tracy, please see comments 27 to 31. QA can't verify this.