Firefox freezes if scrolling option list has specific CSS.

RESOLVED FIXED in mozilla44

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: lyngaas, Assigned: mats)

Tracking

({regression})

18 Branch
mozilla44
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 affected, firefox42 affected, firefox43 affected, firefox44 unaffected, firefox-esr38 affected)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8671122 [details]
firefoxoptionlistcssbug.htm

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.

Comment 1

3 years ago
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
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox-esr38: --- → affected
Component: Untriaged → Layout: Form Controls
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Version: 41 Branch → 18 Branch

Updated

3 years ago
Flags: needinfo?(mats)
(Reporter)

Comment 2

3 years ago
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
(Assignee)

Comment 3

3 years ago
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)

Comment 4

3 years ago
Created attachment 8672344 [details]
reduced
(Assignee)

Comment 6

3 years ago
Created attachment 8672391 [details]
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
(Assignee)

Comment 7

3 years ago
Created attachment 8672392 [details] [diff] [review]
fix

We should drill through any scroll frame before we recurse.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4a124f61035
Attachment #8672392 - Flags: review?(roc)
(Assignee)

Comment 8

3 years ago
Created attachment 8672393 [details] [diff] [review]
crashtest
(Assignee)

Comment 9

3 years ago
BTW, I tested that the patch fixes the problem also without the fix in bug 1208978.
(Assignee)

Comment 10

3 years ago
Created attachment 8672394 [details] [diff] [review]
fix

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+
(Assignee)

Comment 12

3 years ago
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.
(Assignee)

Updated

3 years ago
Flags: in-testsuite+

Comment 14

3 years ago
bugherdermerge
https://hg.mozilla.org/mozilla-central/rev/faa66615c0a1
https://hg.mozilla.org/mozilla-central/rev/090f71020a97
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44

Updated

3 years ago
Depends on: 1228670
You need to log in before you can comment on or make changes to this bug.