Follow up on these comments : https://bugzilla.mozilla.org/show_bug.cgi?id=1183212#c1. Concrete actions to be taken: - Add update badge inside the Hamburger menu popup update button. - Drop all badges on click on the Hamburger menu button. Did I forget anything? CC :shorlander :markh

How it looks now

Comment on attachment 8638842 [details] [diff] [review] bug-1186521.patch Review of attachment 8638842 [details] [diff] [review]: ----------------------------------------------------------------- Looking good, and the screen-shots are what I was expecting to see! ::: browser/base/content/browser.js @@ +2593,5 @@ > PanelUI.menuButton.setAttribute("badge-status", badgeToShow); > if (badgeToShow == "update-failed") { > PanelUI.menuButton.setAttribute("badge", "!"); > } > + PanelUI.panel.addEventListener("popupshowing", this, true); I'm not sure the event listener state is being tracked correctly - eg, multiple addBadge calls, or many removeBadge calls with no corresponding addBadge calls seems likely to screw it up. I don't see a problem with always having the event listener exist (which might even mean you can just have the handler be inline and avoid the handleEvent function and the check it does for the correct event) ::: browser/themes/shared/customizableui/panelUIOverlay.inc.css @@ +542,5 @@ > position: relative; > top: 25%; > } > > +#PanelUI-update-status[update-status="succeeded"]::after { it would be awesome if we could do something clever to avoid duplicating these rules, but I haven't thought too much about how we might go about that. Worth considering though.

Comments addressed!

Comment on attachment 8639985 [details] [diff] [review] bug-1186521.patch Review of attachment 8639985 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me pending shorlander's ui-review in bug 1180584.

Updated to use the new update-failed icon introduced in bug 1180584

Comment on attachment 8643194 [details] [diff] [review] bug-1186521.patch >+#PanelUI-update-status[update-status]::after { >+ content: ""; >+ width: 14px; >+ height: 14px; >+ margin-right: 16.5px; >+ box-shadow: 0px 1px 0px rgba(255, 255, 255, 0.2) inset, 0px -1px 0px rgba(0, 0, 0, 0.1) inset, 0px 1px 0px rgba(12, 27, 38, 0.2); >+ border-radius: 2px; >+ background-size: contain; >+ display: -moz-box; >+} >+ >+#PanelUI-update-status[update-status="succeeded"]::after { >+ background-image: url(chrome://browser/skin/update-badge.svg); >+ background-color: #74BF43; >+} >+ >+#PanelUI-update-status[update-status="failed"]::after { >+ background-image: url(chrome://browser/skin/update-badge-failed.svg); >+ background-color: #D90000; >+} Out of interest, what is this trying to achieve? I ask because I had to remove some uses of ::after in bug 1029937 and I wouldn't want you to trip over the same bug.

Edouard, can you please address comment 8 and see how applicable that bug is here?

I played with the window size and the inspector, wasn't able to reproduce this kind of bug.