overflow: scroll not working in embedded Code Mirror editor
Categories
(Core :: Layout, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr68 | --- | unaffected |
| firefox67 | --- | unaffected |
| firefox68 | --- | unaffected |
| firefox69 | --- | unaffected |
| firefox70 | --- | fixed |
| firefox71 | --- | fixed |
| firefox72 | --- | fixed |
People
(Reporter: rowbot, Assigned: dbaron)
References
(Regression)
Details
(Keywords: regression)
Attachments
(5 files)
|
169.31 KB,
text/html
|
Details | |
|
30.20 KB,
text/html
|
Details | |
|
1.79 KB,
text/html
|
Details | |
|
47 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
|
47 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
STR:
- Visit https://phpstan.org
- In the text area hold down the enter key to create a bunch of new lines so that the text area overflows its containing element.
AR:
The content overflows, but is not scrollable.
ER:
The overflowing content is scrollable.
Regressed by: Bug 1351924
Last good: 2019-07-13
First bad: 2019-07-14
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4fe07886a969323cee79798ec501f43882096a61&tochange=05f675735d39d336dc7253e07872d0232c9ced0e
|
||
Comment 1•5 years ago
|
||
David, do you have time to look?
I think this might be a flexbox issue. There are multiple nested flex boxes that have height: 100%... The key is that inserting a line causes the <div class="CodeMirror-sizer"> to change min-height.
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 2•5 years ago
|
||
|
|
Assignee | |
Comment 3•5 years ago
•
|
||
This one doesn't show quite as much difference -- in the "good" state it doesn't show a scrollbar, but it's still scrollable with the mousewheel. [Edit: that's just because the scrollbar was clipped by the parent element's overflow: hidden.]
|
|
Assignee | |
Comment 4•5 years ago
|
||
|
|
Assignee | |
Comment 5•5 years ago
|
||
So I just looked at a reflow log. middle-column gets reflowed twice, the first time with unconstrained computed height, the second time with 600px computed height. Both times have the v-resize flag set. The problem is that CodeMirror gets reflowed twice, with the same pair of heights, but the second time the v-resize flag is not set, and it optimizes away the reflow of its descendants. This leaves CodeMirror-scroll at its auto-height rather than 600px tall, so there's no scrollability.
|
|
Assignee | |
Comment 6•5 years ago
|
||
So this patch is sufficient to fix the bug here:
diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp
index 4ced3c69c893..14a9930e0f2c 100644
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -5246,6 +5246,7 @@ void nsFlexContainerFrame::ReflowFlexItem(
}
if (didOverrideComputedBSize) {
childReflowInput.SetBResize(true);
+ childReflowInput.mFlags.mIsBResizeForPercentages = true;
}
}
// NOTE: Be very careful about doing anything else with childReflowInput
but the question is whether some or all of the other SetBResize calls need the same treatment. (I should also look at the grid code to see what it needs.)
Given the schedule, I think it's probably best to back out bug 1351924 for 70 and try to fix this on nightly for 71. I should file a separate bug for that.
|
|
Assignee | |
Comment 7•5 years ago
|
||
I filed bug 1586175 to back bug 1351924 out for Firefox 70 (only) in order to give us more time to fix this regression.
|
|
Assignee | |
Comment 8•5 years ago
|
||
Note that the key factor for making the decisions about whether that added line is necessary is whether this is a resize that can change what percentages (for height/min-height/max-height) mean in the child.
|
||
Comment 9•5 years ago
|
||
| bugherder uplift | ||
|
||
Updated•5 years ago
|
|
||
Comment 10•5 years ago
|
||
David, what is the status regarding Firefox 71 (with the context that we are entering in beta next Monday)? Thanks
|
|
Assignee | |
Comment 11•5 years ago
|
||
|
|
Assignee | |
Comment 12•5 years ago
|
||
|
|
Assignee | |
Comment 13•5 years ago
|
||
Here's a try run.
|
|
Assignee | |
Comment 14•5 years ago
|
||
Here's a new try run.
|
||
Comment 15•5 years ago
|
||
Pushed by dbaron@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/28980de08edf Add reftest. r=dholbert https://hg.mozilla.org/integration/autoland/rev/5ea7c20012ed Make flex and grid code indicate that its block resizes can affect basis for percentages. r=dholbert,mats
|
||
Comment 16•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/28980de08edf
https://hg.mozilla.org/mozilla-central/rev/5ea7c20012ed
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/19883 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
|
|
Assignee | |
Comment 19•5 years ago
•
|
||
Comment on attachment 9104030 [details]
Bug 1584018 - Make flex and grid code indicate that its block resizes can affect basis for percentages.
Beta/Release Uplift Approval Request
- User impact if declined: layout bugs such as the one here, where a user can't scroll to the content
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): this change is pretty low risk; it's essentially disabling a new optimization from bug 1351924 in a group of cases where it's not valid
- String changes made/needed: no
|
|
Assignee | |
Updated•5 years ago
|
|
|
||
Comment 20•5 years ago
|
||
Comment on attachment 9104030 [details]
Bug 1584018 - Make flex and grid code indicate that its block resizes can affect basis for percentages.
P1, has tests, fix evaluated as low risk, uplift approved for 71 beta 5, thanks!
|
|
||
Updated•5 years ago
|
|
||
Comment 21•5 years ago
|
||
| bugherder uplift | ||
|
||
Updated•5 years ago
|
|
||
Updated•2 years ago
|
Description
•