Closed Bug 1218797 Opened 5 years ago Closed 2 years ago

Toolbar-Button flickers on being pressed

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: designakt, Unassigned)

Details

Attachments

(1 file)

Attached image pressedButtonTest.gif
When changing the state of the hello button in the toolbar to pressed (to open the panel), the button outline changes from thin to thick, back to thin and back to thick again, before staying that way for the time it is pressed.
This behavior does not exist on the menu- and download-buttons.
I think from your gif that this also applies to the whimsy button. Therefore I don't think this is hello specific, but more a general Firefox bug, so moving to a more appropriate product/component.

It could be something to do with `badged-button`s from a quick look in inspector.
Component: Client → Toolbars and Customization
Product: Hello (Loop) → Firefox
CCing Blake as he fixed it for Whimsy (and WebExtensions in general) today (bug 1217129).
This leaves Hello being the only button with the flickering, but he might know how to fix it.
It started off not showing the pressed outline at all, so the problem with Whimsy/WebExtensions was one of my own making.  The fix for that was to move the:
    node.setAttribute("open", "true");
line up to https://reviewboard.mozilla.org/r/22945/diff/6#chunk2.3

I'm not sure where that attribute is being set in the Hello code, but I suspect it should be quite a bit earlier than it is.
(I tried looking for it for 10-20 minutes, but none of the things I found seemed to be called by the panel opening code.  Maybe the problem is that you're removing the attribute and then re-adding it?  I'm just guessing here, though…)
(Adding Standard8, cause it really is a Hello/Loop bug, not a general Firefox bug…  ;)
Well here's a few relevant bits of code, but I don't see anything that is setting the open attribute. So I'm guessing this could be something in CustomizableWidget, but then I don't know why other panels aren't seeing it.

http://hg.mozilla.org/mozilla-central/annotate/cc48981c026c/browser/base/content/browser-loop.js#l74
http://hg.mozilla.org/mozilla-central/annotate/cc48981c026c/browser/components/customizableui/CustomizableWidgets.jsm#l955

cc'ing a couple of more folks who might have an idea.
(In reply to Blake Winton (:bwinton) (:☕️) from comment #3)
> It started off not showing the pressed outline at all, so the problem with
> Whimsy/WebExtensions was one of my own making.  The fix for that was to move
> the:
>     node.setAttribute("open", "true");
> line up to https://reviewboard.mozilla.org/r/22945/diff/6#chunk2.3
> 
> I'm not sure where that attribute is being set in the Hello code, but I
> suspect it should be quite a bit earlier than it is.
> (I tried looking for it for 10-20 minutes, but none of the things I found
> seemed to be called by the panel opening code.  Maybe the problem is that
> you're removing the attribute and then re-adding it?  I'm just guessing
> here, though…)

The attribute ought to be set and removed by the XUL popup code. I don't know offhand why it wasn't in Blake's testing, but JS shouldn't be messing with it manually. Fixing that for both hello (which does it here:

https://dxr.mozilla.org/mozilla-central/source/browser/modules/PanelFrame.jsm

) and the extension stuff should fix this issue, and will likely make the panels work better than they currently do anyway...
Should the XUL popup code also be handling stuff like bug 1217129?
(In reply to Blake Winton (:bwinton) (:☕️) from comment #7)
> Should the XUL popup code also be handling stuff like bug 1217129?

Yes. That's quite possibly another consequence of manually messing with this attribute.
Hello went away, so I don't think this is an issue anymore.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.