If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 29

Status

()

Firefox
Menus
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: u498184, Assigned: jaws)

Tracking

(Depends on: 1 bug, Blocks: 3 bugs, {regression})

29 Branch
Firefox 30
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29 fixed, firefox30 fixed)

Details

(Whiteboard: [Australis:P2])

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 8374411 [details]
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.
(Reporter)

Updated

4 years ago
Component: Untriaged → Menus
(Reporter)

Comment 1

4 years ago
Created attachment 8374413 [details]
australis stylish and https.png
Blocks: 939862, 872617
Whiteboard: [Australis:P?]

Comment 2

4 years ago
(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?
Blocks: 942157
No longer blocks: 872617
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(nothingfinerthanscottsteiner)
Keywords: regression, regressionwindow-wanted
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Add-on icons incorrectly sized on menu panel → HTTPS everywhere, stylish, and noscript icons incorrectly sized on menu panel
(Reporter)

Comment 3

4 years ago
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.
Flags: needinfo?(nothingfinerthanscottsteiner)

Comment 4

4 years ago
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
Blocks: 897496
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.
Whiteboard: [Australis:P?] → [Australis:P2]

Comment 6

4 years ago
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
Removing keyword since regression was noted in comment #4. Thanks Alice.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Keywords: regressionwindow-wanted

Updated

4 years ago
Depends on: 941045
Blocks: 976420
No longer blocks: 942157
Updating the summary based on comment #2 and the wontfix'ing of bug 941045.
Summary: HTTPS everywhere, stylish, and noscript icons incorrectly sized on menu panel → Noscript icon is incorrectly sized on menu panel

Comment 9

4 years ago
(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.
Created attachment 8383903 [details] [diff] [review]
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.
Attachment #8383903 - Flags: review?(mconley)
Created attachment 8383923 [details]
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.

Updated

4 years ago
Duplicate of this bug: 978263
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.
Attachment #8383903 - Flags: review?(mconley) → review+
needinfo'ing about getting in touch with the Stylish author(s).
Flags: needinfo?(jaws)
(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

4 years ago
Clearing needinfo per comment #15, updating summary.
Flags: needinfo?(jaws)
Summary: Noscript icon is incorrectly sized on menu panel → Fix styling of type="menu" buttons in the menu panel

Updated

4 years ago
Duplicate of this bug: 973243

Updated

4 years ago
Blocks: 467520
https://hg.mozilla.org/integration/fx-team/rev/ed3e48147817
https://hg.mozilla.org/mozilla-central/rev/ed3e48147817
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30

Comment 20

4 years ago
Created attachment 8385357 [details]
A button of type="menu" (Language), after patch, Linux

In Linux Nightly, no dropmarker is visible with this patch.
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?
Flags: needinfo?(nohamelin)
The add-on buttons seem also to be taller than the (already big) classic buttons in the menu panel.

Updated

4 years ago
Depends on: 979610

Comment 23

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

This bug will be backported to Aurora?
Flags: needinfo?(nohamelin)
If it is fixed in time to backport to Aurora, then yes :)
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.
Attachment #8383903 - Flags: approval-mozilla-aurora?
status-firefox29: --- → affected
status-firefox30: --- → fixed
Attachment #8383903 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/d40e9c054bc3
status-firefox29: affected → fixed

Updated

4 years ago
QA Contact: cornel.ionce

Comment 27

4 years ago
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?
Flags: needinfo?(nothingfinerthanscottsteiner)
QA Contact: cornel.ionce
(Reporter)

Comment 28

4 years ago
• 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)
Flags: needinfo?(nothingfinerthanscottsteiner)
(Reporter)

Comment 29

4 years ago
Created attachment 8402745 [details]
australis menu bug.png
Attachment #8374411 - Attachment is obsolete: true
Attachment #8374413 - Attachment is obsolete: true
(Reporter)

Comment 30

4 years ago
Reopened because it isn't resolved
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
Depends on: 993208
No longer depends on: 993208

Updated

4 years ago
Keywords: verifyme

Comment 32

3 years ago
Tracy, please see comments 27 to 31. QA can't verify this.
Keywords: verifyme

Updated

3 years ago
QA Whiteboard: [qa-]

Updated

3 years ago
Depends on: 1025967

Updated

a year ago
Depends on: 1276956
You need to log in before you can comment on or make changes to this bug.