The code currently managing the customizable zoom control is a bit more messy than the URLBarZoom.jsm code and maybe even suffers from subtle bugs still. Now, rather than copying the code from one place to the other, we should just share it.

(sorry, I'm out today and won't be able to review it today)

Comment on attachment 8848264 [details] Bug 1348122 - Share code between the customizable zoom control and the location bar's zoom indicator. https://reviewboard.mozilla.org/r/121202/#review123814 ::: browser/modules/FullZoomUI.jsm:12 (Diff revision 1) > > -this.EXPORTED_SYMBOLS = [ "URLBarZoom" ]; > +this.EXPORTED_SYMBOLS = [ "FullZoomUI" ]; > > Components.utils.import("resource://gre/modules/Services.jsm"); > > -var URLBarZoom = { > +let windows = []; Use a const Set instead? Then you can use the more idiomatic .add and .delete to add/remove items. ::: browser/modules/FullZoomUI.jsm:108 (Diff revision 1) > +customizationListener.onWidgetAdded = > +customizationListener.onWidgetRemoved = > +customizationListener.onWidgetMoved = > +customizationListener.onWidgetInstanceRemoved = function(aWidgetId) { > + if (aWidgetId == "zoom-controls") { > + for (let window of windows) { In fact, rather than keeping our own set of windows, you could just iterate over CustomizableUI.windows directly.

Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c8f8b6cb5b7b Share code between the customizable zoom control and the location bar's zoom indicator. r=Gijs

Please request Aurora approval on this when you get a chance (jaws mentioned that you wanted to uplift this when we were discussing bug 1318830 and bug 1345375).

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7) > Please request Aurora approval on this when you get a chance (jaws mentioned > that you wanted to uplift this when we were discussing bug 1318830 and bug > 1345375). That's a bit of a mischaracterization. Jared wanted to uplift bug 1345375 to ease potential future uplifts. This is only going to pan out if we uplift this too.

Approval Request Comment [Feature/Bug causing the regression]: bug 565718 & bug 869933 [User impact if declined]: / [Is this code covered by automated tests?]: yes, to some extent [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: / [Is the change risky?]: average risk level [Why is the change risky/not risky?]: there's nothing particularly risky about this patch [String changes made/needed]: the original patch has string changes. I've removed them in this patch.

Hi ::dao, From your uplift request, there is no user impact. Why do we need this in Aurora54? Can we let this ride the train on 55 and won't fix in 54?

(In reply to Gerry Chang [:gchang] from comment #10) > Hi ::dao, > From your uplift request, there is no user impact. Why do we need this in > Aurora54? Can we let this ride the train on 55 and won't fix in 54? Please see comment 8.

Comment on attachment 8851945 [details] [diff] [review] patch for uplift Take this to ease potential future uplifts. Aurora54+.

https://hg.mozilla.org/releases/mozilla-aurora/rev/0032f065b456fef31bc6deb4ead372dd8faa9611