1178422 - Badged menu-buttons on Windows don't highlight

Status: RESOLVED WONTFIX

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Kaply [:mkaply]]

If you create a badged menu-button, hovering over it doesn't show the highlight.

I have a patch for this.

Comment 1

[Ben Bucksch (:BenB)]

> I have a patch for this.

That's nice. Can we have it, too? ;-)

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Add in missing cases

(In reply to Ben Bucksch (:BenB) from comment #1)
> > I have a patch for this.
> 
> That's nice. Can we have it, too? ;-)

Where do you think the patch came from? :)


This adds in the missing cases. Basically anywhere there was a .toolbarbutton-menubutton-button > .toolbarbutton-icon, it should have had a .toolbarbutton-menubutton-button > .toolbarbutton-badge-container as well.

Comment 3

[:Gijs (he/him)]

I'm sorry for not reviewing this within my normal deadline of 24 hours, I'm still getting out of whistler backlog (and having to deal with MS releasing win10 updates all the time).

Do you have an add-on that I can test this with? The CSS looks fine, but I'd like to actually see it apply to something, and I don't have usable badged menu buttons.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8627367 [details] [diff] [review]
Add in missing cases

It's hard to be sure without a testcase, but I expect this will also need changes in panelUIOverlay.(inc.)css , and maybe devedition CSS to not break stuff just in that theme.

Comment 5

[Mike Kaply [:mkaply]]

> Do you have an add-on that I can test this with? The CSS looks fine, but I'd like to actually see it apply to something, and I don't have usable badged menu buttons.

I'll get a test add-on together.

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Test XPI

Here's an XPI to test with.

It adds a toolbar and nav buttons to compare.

Underneath the menus are menuitems that let you remove the badges to compare.

Not sure if you are using HiRes Windows or not, but there is some weirdness around the icons on the nav bar disappearing when you turn off the badges. Investigating that as a separate issue.

Comment 7

[Mike Kaply [:mkaply]]

And this only works on nightly. For some reason on FF38, adding badges to any button with  menus makes the dropmarker disappear? I'm at a loss.

Comment 8

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #7)
> And this only works on nightly. For some reason on FF38, adding badges to
> any button with  menus makes the dropmarker disappear? I'm at a loss.

Didn't the badged-menubutton support not land until quite recently, so it's not in Fx38? bug 1157688 is marked fixed in 40, but not 39 or 38, so that makes sense to me...

(In reply to :Gijs Kruitbosch from comment #4)
> It's hard to be sure without a testcase, but I expect this will also need
> changes in panelUIOverlay.(inc.)css , and maybe devedition CSS to not break
> stuff just in that theme.

Were you planning on changing the patch to address this feedback?

Comment 9

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #8)
> (In reply to :Gijs Kruitbosch from comment #4)
> > It's hard to be sure without a testcase, but I expect this will also need
> > changes in panelUIOverlay.(inc.)css , and maybe devedition CSS to not break
> > stuff just in that theme.
> 
> Were you planning on changing the patch to address this feedback?

(just so we're clear, not trying to push, just wondering if I need to re-review right now or wait)

Comment 10

[Mike Kaply [:mkaply]]

> Didn't the badged-menubutton support not land until quite recently, so it's not in Fx38? bug 1157688 is marked fixed in 40, but not 39 or 38, so that makes sense to me...

Yes, but even regular menu buttons get their dropmarkers dropped. It's weird.

> Were you planning on changing the patch to address this feedback?

Yes. Let me see if anything else is needed before reviewing.

tx

Comment 11

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #10)
> > Didn't the badged-menubutton support not land until quite recently, so it's not in Fx38? bug 1157688 is marked fixed in 40, but not 39 or 38, so that makes sense to me...
> 
> Yes, but even regular menu buttons get their dropmarkers dropped. It's weird.

Err, that's a regression, I'm 99% sure - we did quite a bit of work to make that work for Australis... Can you file a separate bug and needinfo me? (bonus points if you have an easy testcase for a non-badged menu button as well...)

> > Were you planning on changing the patch to address this feedback?
> 
> Yes. Let me see if anything else is needed before reviewing.

Perfect, thanks!

Comment 12

[:Gijs (he/him)]

bug 1029937 is going to bitrot things here, but it might help you a bit. Hopefully.

Comment 13

[Mike Kaply [:mkaply]]

(In reply to :Gijs Kruitbosch from comment #12)
> bug 1029937 is going to bitrot things here, but it might help you a bit.
> Hopefully.

Yeah, I'm quite happy with Neil's changes. Should make things a little better.

Comment 14

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.

Comment 15

[Mike Kaply [:mkaply]]

Related to old style extensions.