Closed Bug 1251902 Opened 4 years ago Closed 4 years ago

"Hello" and "Share this page" buttons sometimes blink when I click them to open panel

Categories

(Hello (Loop) :: Client, defect, P3)

defect

Tracking

(firefox47 affected, firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- affected
firefox48 --- fixed

People

(Reporter: arni2033, Assigned: standard8)

References

Details

(Whiteboard: [btpp-fix-later])

Attachments

(1 file, 1 obsolete file)

>>>   My Info:   Win7_64, Nightly 47, 32bit, ID 20160225030209
STR:
1. Move "Share this page" and "Hello" buttons to navigation toolbar
2. Open page https://www.mozilla.org/en-US/ (somehow heavy for "share this page")
3. Click "Share this page" / "Hello"  toolbarbutton

AR:
 The toolbarbutton blinks white-black-white-black:
 First it's white, because mouse pointer is placed above
 -> then it's black, because it's pressed
 -> then it's white, because I released left mouse button, but it's still above the button
 -> then it's black, because widget panel is open

ER:  
 Just like all other built-in buttons: only "white->black" change of color when I click button

Notes:
 This doesn't happen with add-ons: uBlock, Tab Data, Simplified Tab Groups. I haven't looked precisely
 on how those 2 toolbarbuttons are implemented, but probably this could be helped even on "Theme" side
 I hope that somebody will assign the right component.

Screencast (you can see both types of behavior - AR for Hello and ER for others)
> ‎(2015.10.29 13-55-16)   https://dl.dropboxusercontent.com/s/i7s55z1gle0n19r/screencast%201%20-%20Widget%20icons%20sometimes%20blink.webm?dl=0
(In reply to arni2033 from comment #0)

> Just like all other built-in buttons: only "white->black" change of color when I click button

Interesting. This behavior is easy to see in bookmarks toolbar. (button-down > button-up > button-down)
In step 1, Move "Share this page" and "Hello" buttons to bookmarks toolbar
I expect the issue is that the 'open' attribute gets set much later for these buttons, or something along those lines - it seems to work fine on most other buttons, so moving this to loop for them to diagnose. Once we have a diagnosis it should be possible to apply that to the share page button; either way it doesn't seem plausible there's a theme issue considering it works with the other buttons.
Component: Theme → Client
Product: Firefox → Hello (Loop)
Version: Trunk → unspecified
Rank: 35
Priority: -- → P3
Whiteboard: [btpp-fix-later]
This is a prototype I've whipped up - the issue is in the social code. I think as Gijs said, we're just setting the open attribute way too late.

This should fix it, though I'm doing a bit more testing before getting review.
Assignee: nobody → standard8
FWIW, I've never understood why this code needs to set the attribute "manually", as it were - the attribute is normally set (and removed) on its anchor by the panel, I believe.
Comment on attachment 8725631 [details] [diff] [review]
Stop the 'Hello' and 'Share this page' buttons blinking when the panel is being opened.

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

I'm not sure why there's no automatic updating here. Maybe Shane knows?

I've tried this a few more times and it seems fine to fix the bug at least (and would probably be good for uplift). Obviously, if there's a way to get automatic, we could deal with that in a follow-up.
Attachment #8725631 - Flags: review?(mixedpuppy)
Comment on attachment 8725631 [details] [diff] [review]
Stop the 'Hello' and 'Share this page' buttons blinking when the panel is being opened.

tbh I don't recall why it's necessary to set the attribute.  I'd have to spend some time digging into that issue.  I think this is a fine fix for now.
Attachment #8725631 - Flags: review?(mixedpuppy) → review+
Shane: Try server revealed that the patch broke the browser_UITour_loop_panel test. This adjusts the test to on the panel state to be open rather than the button state, since we're adjusting where these opens are set.
Attachment #8728006 - Flags: review?(mixedpuppy)
Attachment #8725631 - Attachment is obsolete: true
Attachment #8728006 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/integration/fx-team/rev/d3b650bd44960ba00e28a0a6b539828c0179ccf9
Bug 1251902 - Stop the 'Hello' and 'Share this page' buttons blinking when the panel is being opened. r=mixedpuppy
https://hg.mozilla.org/mozilla-central/rev/d3b650bd4496
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
See Also: → 1256056
You need to log in before you can comment on or make changes to this bug.