Closed Bug 536945 Opened 10 years ago Closed 10 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3.7a1

People

(Reporter: dao, Assigned: dao)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [tsnap])

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #419291 - Flags: review?(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 []?
Attachment #419291 - Flags: review?(mstange) → review+
I thought [] would complain if the index was >= the length, but apparently it doesn't.
(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])
I get the warning here:
> document.createElement('label').childNodes[0]

but not here:
> var foo = document.createElement('label').childNodes[0]
http://hg.mozilla.org/mozilla-central/rev/48e1125a25ee
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
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.
(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
Depends on: 537037
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.
Attachment #419291 - Flags: approval1.9.2.1?
Depends on: 540102
Attachment #419291 - Flags: approval1.9.2.1?
Depends on: 545583
You need to log in before you can comment on or make changes to this bug.