1374257 - label-control_XBL_Constructor is visible on startup profiles

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, enhancement, P1)

Description

[Florian Quèze [:florian]]

Here is a startup profile where label-control_XBL_Constructor takes 19ms before first paint http://bit.ly/2sOkF0g despite no XUL label being visible on screen.

This binding is used because of the CSS rule at:
http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/toolkit/content/minimal-xul.css#67 for label.toolbarbutton-multiline-text

Our toolbarbutton bindings use the toolbarbutton-multiline-text class: http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/toolkit/content/widgets/toolbarbutton.xml#23

We then have another CSS rule that hides these labels (display:none) http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/toolkit/content/xul.css#156

So I think it was assumed that due to the display: none, the constructor would not be called and this would be "free". But I've recently noticed that just accessing a dom node from JS (eg. a getElementById, or .firstChild) forces the binding to be attached, even if the node is hidden.

Adding a dump() in the label-control constructor shows that we currently call this code about 20 times during startup.

Comment 1

[Florian Quèze [:florian]]

Attachment: Patch

Setting -moz-binding to none at the same time as display: none is enough to avoid the cost of this constructor, and here is a trivial patch doing it. I'm confident that it fixes the performance aspect, but I'm not fully confident that it won't have unintended side effects (I haven't noticed any while quickly testing the patch locally).

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e337301d8abd
avoid attaching an xbl binding to hidden labels, r=dao.

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e337301d8abd