536945 - Implement wrapping of tab previews in JS to avoid performance issues with display:block

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

spin-off from bug 528531 comment 4:
> For the slow mouseover feedback performance I've found that removing
> display:block makes it a lot faster. Maybe we'll have to reimplement the
> wrapping in JavaScript - mixing display:block with XUL doesn't seem to work
> very well.

Comment 1

[Markus Stange [:mstange]]

Comment on attachment 419291 [details] [diff] [review]
patch

Looks good and fixes the mouseover performance issues for me completely.

>+        row = this.container.childNodes.item(Math.floor(++i / this._columns));

Out of curiosity, why .item() and not []?

Comment 2

[Dão Gottwald [:dao]]

I thought [] would complain if the index was >= the length, but apparently it doesn't.

Comment 3

[[e-mail]]

(In reply to comment #2)
> I thought [] would complain if the index was >= the length, but apparently it
> doesn't.
It should be a strict JavaScript warning. (e.g. document.createElement('label').childNodes[0])

Comment 4

[Dão Gottwald [:dao]]

I get the warning here:
> document.createElement('label').childNodes[0]

but not here:
> var foo = document.createElement('label').childNodes[0]

Comment 5

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/48e1125a25ee

Comment 6

[Boris Zbarsky [:bzbarsky]]

Can you please file a layout bug, ideally with a testcase, on the slow-performance situation this worked around?  It's not clear to me why display:block should be causing problems here.

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #6)
> Can you please file a layout bug, ideally with a testcase, on the
> slow-performance situation this worked around?  It's not clear to me why
> display:block should be causing problems here.

filed bug 537037

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 419291 [details] [diff] [review]
patch

Requesting approval for an isolated change to visibly improve performance when interacting with the all-tabs panel. Only affects users who've toggled the hidden browser.allTabs.previews pref.