Closed Bug 1212688 Opened 10 years ago Closed 10 years ago

Firefox freezes if scrolling option list has specific CSS.

Categories

(Core :: Layout: Form Controls, defect)

18 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected
firefox44 --- unaffected
firefox-esr38 --- affected

People

(Reporter: lyngaas, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: regression)

Attachments

(5 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 Build ID: 20150929144111 Steps to reproduce: This CSS: * {overflow-x: hidden;} causes firefox to freeze if you make a <select> list with 17 or more <option>'s Actual results: Putting 17 items in a select list display fine, but when you put 18 options in and click the select menu it freezes firefox and slows it to a crawl (instead of triggering a vertical scrollbar). Expected results: The overflow-x CSS should have just prevented horizontal scrollbars on everything, and not frozen the page.
I can reproduce 100% cpu usage and significant response performance degradation. Regressed since Firefox18. Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=533faa3c50ed&tochange=b963ae0f9b60 Regressed by: Bug 806364
Blocks: 806364
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: Form Controls
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Version: 41 Branch → 18 Branch
Flags: needinfo?(mats)
NOTE from further observation: The bug triggers at 20 items in a select list, not 18 or 17. I have an extra option above my option group I didn't notice when I submitted, and the option group itself should count as one item since it takes up one line. Still it freezes when it hits the scrolling point for a <select> list. Thanks
A minimal testcase would be nice. In particular, which element needs to have "overflow-x: hidden" for it to occur? FWIW, I can reproduce it in Aurora(v43), but not in Nightly(v44), both on Linux64. (Maybe we optimize away reflows in Nightly in some situations that masks it?)
Flags: needinfo?(mats)
Attached file reduced
Attached file frame tree
The problem is the <optgroup> scroll frame. When GetMaxOptionBSize finds a frame with <optgroup> content it recurses into its primary frame: http://hg.mozilla.org/mozilla-central/annotate/b68eab795f9d/layout/forms/nsListControlFrame.cpp#l265 which is the scroll frame in this case, and the first child of that is the scrollbar frame which has the block-size large enough to fit all its <option> children. So we overestimate the row size by a huge amount. Apparently, that sends us into a reflow loop. Bug 1208978 wallpapers that by limiting the size, breaking the loop. But that seems mostly like a coincidence.
Assignee: nobody → mats
Attached patch fix (obsolete) — Splinter Review
We should drill through any scroll frame before we recurse. https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4a124f61035
Attachment #8672392 - Flags: review?(roc)
Attached patch crashtestSplinter Review
BTW, I tested that the patch fixes the problem also without the fix in bug 1208978.
Attached patch fixSplinter Review
Right fix this time. (shouldn't clobber |option| since that's used for the loop) https://treeherder.mozilla.org/#/jobs?repo=try&revision=50ac9e3b2a4a
Attachment #8672392 - Attachment is obsolete: true
Attachment #8672392 - Flags: review?(roc)
Attachment #8672394 - Flags: review?(roc)
Comment on attachment 8672394 [details] [diff] [review] fix Review of attachment 8672394 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. However, shouldn't we also prevent scrollframe construction on descendants of a <select>? That doesn't make sense to me and could break other things. I guess that would require a change to nsCSSFrameConstructor? But we should take this patch anyway since it's simple and correct in itself.
Attachment #8672394 - Flags: review?(roc) → review+
Yeah, we should probably just ignore a bunch of properties on <option>/<optgroup>. A quick test in Chrome shows that it ignores 'overflow' and 'height' for example. Filed bug 1213819.
Flags: in-testsuite+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1228670
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: