Closed Bug 1071635 Opened 5 years ago Closed 5 years ago

checkSizing() and onPageFirstSized() cause uninterruptible reflows

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 35
Iteration:
35.2

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

By accessing |document.documentElement.clientWidth| and calling |.getBoundingClientRect()| for all hidden elements (+ 1) about:newtab causes uninterruptible reflows when the page is first shown.

1) Code that collects telemetry data should not have any performance impact.
2) There might be a different/better way to determine how many children of #newtab-grid are visible?
Blocks: 1071638
Looks like nsIDOMWindowUtils has everything we need.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8493854 - Flags: review?(adw)
Comment on attachment 8493854 [details] [diff] [review]
0001-Bug-1071635-Get-rid-of-uninterruptible-reflows-cause.patch

Sorry, forgot to run tests. Will need to make some adjustments.
Attachment #8493854 - Flags: review?(adw)
Passes tests now. Turns out I was actually calculating the wrong index by not ignoring empty cells.
Attachment #8493854 - Attachment is obsolete: true
Attachment #8493919 - Flags: review?(adw)
Looks like this snuck in with bug 1042214. The changes to browser_tabopen_reflow.js which obviously caught this have unfortunately not been reviewed :/
Blocks: 1042214
(In reply to Tim Taubert [:ttaubert] from comment #4)
> Looks like this snuck in with bug 1042214. The changes to
> browser_tabopen_reflow.js which obviously caught this have unfortunately not
> been reviewed :/
Sorry about that. I thought it was explicit that we wanted the reflow to calculate positions for the functionality, so the test was updated for that.
Yeah, I assumed this was a quick fix to unbreak the test but we don't actually want sync reflows :) I could maybe add something to the test that explicitly states what the test tries to prevent and who to ask for review? The test itself isn't that clear I think.
Attachment #8493919 - Flags: review?(adw) → review+
Iteration: --- → 35.2
Points: --- → 3
QA Whiteboard: [qa-]
Flags: firefox-backlog+
QA Whiteboard: [qa-]
Flags: qe-verify-
https://hg.mozilla.org/mozilla-central/rev/f191b431571c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Blocks: 1050643
Depends on: 1135285
You need to log in before you can comment on or make changes to this bug.