This is a required step of bug 1450823.

Comment on attachment 8976483 [details] Bug 1462297 - Remove the contents of the "ctrlTab-preview" binding. https://reviewboard.mozilla.org/r/244608/#review250686 ::: browser/base/content/browser-ctrlTab.js:256 (Diff revision 1) > + let previewInner = document.createElement("vbox"); > + previewInner.setAttribute("class", "ctrlTab-preview-inner"); > + preview.appendChild(previewInner); It might be worth checking if the inner box is still needed. I think it might be possible to set -moz-box-orient: vertical; on the parent button, and remove the child vbox. ::: browser/base/content/browser-ctrlTab.js:267 (Diff revision 1) > + canvas.setAttribute("style", "max-width:" + canvasWidth + "px;" + > + "min-width:" + canvasWidth + "px;" + > + "max-height:" + canvasHeight + "px;" + > + "min-height:" + canvasHeight + "px;"); canvas.style.maxWidth = canvas.style.maxHeight = ... might be more elegant.

Comment on attachment 8976483 [details] Bug 1462297 - Remove the contents of the "ctrlTab-preview" binding. https://reviewboard.mozilla.org/r/244608/#review250922 If we were starting this feature from scratch today I think we could simplify how this is done quite a bit, but given that it's not even on by default it's not worth spending a bunch of time refactoring it as we go. ::: browser/base/content/browser-ctrlTab.js:238 (Diff revision 1) > }, > observe(aSubject, aTopic, aPrefName) { > this.readPref(); > }, > > + makePreview: function ctrlTab_makePreview(aIsShowAllButton) { I guess we are safe from the footguns that `parseXULToFragment` around XBL construction that is meant to avoid in this particular case, since we don't call/set anything on the XBL-bound elements being constructed here (button and label)? ::: browser/base/content/browser-ctrlTab.js:264 (Diff revision 1) > + > + if (!aIsShowAllButton) { > + let canvasWidth = this.canvasWidth; > + let canvasHeight = this.canvasHeight; > + > + let canvas = preview._canvas = document.createElement("hbox"); I don't love setting these expando properties on the nodes, but if we can figure out the focus/tabbing issues in Bug 1450823 then we should be able to convert this to a CE (registered only for this document and when the feature is enabled). Which would then let us move the DOM setup and getters onto that class. ::: browser/base/content/browser-tabPreviews.xml:10 (Diff revision 1) > - > <bindings id="tabPreviews" > xmlns="http://www.mozilla.org/xbl" > xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > xmlns:xbl="http://www.mozilla.org/xbl"> > <binding id="ctrlTab-preview" extends="chrome://global/content/bindings/button.xml#button-base"> Would it be possible to directly bind these elements to button-base and remove this file instead of having a pretty much empty binding? Or do we need this for `<content/>`?

(In reply to :ntim (busy until May 25th) from comment #2) > It might be worth checking if the inner box is still needed. I think it > might be possible to set -moz-box-orient: vertical; on the parent button, > and remove the child vbox. It looks like it's needed for providing the correct spacing. It may be possible to remove it and do thing differently, but as Brian mentioned it's probably not worth the effort right now. I explored a few refactoring opportunities like this, but they would result in too many changes. I updated the code to use the style properties instead of the attribute. > I guess we are safe from the footguns that `parseXULToFragment` around XBL > construction that is meant to avoid in this particular case, since we don't > call/set anything on the XBL-bound elements being constructed here (button > and label)? Right, we only set attributes on the elements with bindings, so we should be fine. > Would it be possible to directly bind these elements to button-base and > remove this file instead of having a pretty much empty binding? Or do we > need this for `<content/>`? I thought we wouldn't want to bind to button-base, but that actually works fine here.

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/84a09a4a0dc2 Remove the "ctrlTab-preview" binding. r=bgrins

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a22438592c Fix leftover reference to "browser-tabPreviews.xml". rs=Aryx on a CLOSED TREE

https://hg.mozilla.org/mozilla-central/rev/84a09a4a0dc2 https://hg.mozilla.org/mozilla-central/rev/e1a22438592c