Closed Bug 971246 Opened 6 years ago Closed 6 years ago
Fix styling of type="menu" buttons in the menu panel
1.27 KB, patch
|Details | Diff | Splinter Review|
5.86 KB, image/png
5.03 KB, image/png
17.76 KB, image/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.
(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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
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.
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
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]
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
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
(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.
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)
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 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).
(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.
Clearing needinfo per comment #15, updating summary.
Summary: Noscript icon is incorrectly sized on menu panel → Fix styling of type="menu" buttons in the menu panel
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
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?
The add-on buttons seem also to be taller than the (already big) classic buttons in the menu panel.
Ok, it's bug 979610. The misalignment of the button needs be covered by other bug. This bug will be backported to Aurora?
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?
Attachment #8383903 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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?
QA Contact: cornel.ionce
• 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)
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
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
No longer depends on: 993208
Tracy, please see comments 27 to 31. QA can't verify this.
You need to log in before you can comment on or make changes to this bug.