Some JS-created dropdowns disappear, only with body { display: table }, since bug 1102175
Categories
(Core :: Layout, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr68 | --- | unaffected |
| firefox75 | --- | wontfix |
| firefox76 | --- | wontfix |
| firefox77 | --- | fixed |
People
(Reporter: emilio, Assigned: TYLin)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files, 2 obsolete files)
Over in bug 1600815 comment 16, Paul mentioned:
I have been having a very similar problem and have narrowed it down with Mozregression.
They did share a URL, and I could confirm their regression range.
Their problem seems to be unrelated to bug 1600815, in the sense that it's not a native <select> element, but some sort of javascript library with logic to show / hide the dropdown with jQuery.
The regression range seems to point to bug 1102175, and it seems plausible. There's a <body> element with display: table, and Paul mentioned that switching it to display: flex did fix their issue.
So, two questions:
-
Paul, do you know which library you're using to display the "fake" dropdowns? It's definitely not the native UI.
-
Ting-Yu, maybe you can take a look at the URL to see if something stands out?
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Comment 1•4 years ago
|
||
So I dug a bit, and the library auto-hides the popup when we get the "scroll" event. But its process of showing the popup somehow causes us to fire a scroll event.
|
|
Reporter | |
Comment 2•4 years ago
|
||
(Same regression range)
|
|
Reporter | |
Comment 3•4 years ago
|
||
So this is indeed due to the unconditional reframing of the root when the body gets reframed...
Not quite sure what the right fix for this is yet. Thoughts Ting-Yu?
|
|
Reporter | |
Comment 4•4 years ago
|
||
Well ignore the description of the test-case, behaves badly in both situations ;)
|
|
Reporter | |
Comment 5•4 years ago
|
||
Here is a stripped back jsfiddle of the problem https://jsfiddle.net/a4jg9ekd/7/
Indeed it does seem to be related to Kendo UI Jquery library, removing the library results in the select behaving correctly.
To resolve the issue in the jsfiddle, simple change the body, body form style of display: table to display: block.
|
||
Updated•4 years ago
|
|
||
Updated•4 years ago
|
|
Assignee | |
Comment 7•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
Created attachment 9141140 [details]
Slightly better test-case.So this is indeed due to the unconditional reframing of the root when the body gets reframed...
Not quite sure what the right fix for this is yet. Thoughts Ting-Yu?
To be specific, the unconditional reframing of the root when body gets reframed is at the beginning in nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval.
I think the code can be reached in three circumstances.
- The
<body>element is removed viansCSSFrameConstructor::ContentRemoved. - Restyle manager calls
RecreateFramesForContentto reconstruct<body>because some styles likewriting-modeare changed. - Some contents are inserted into
<body>which has a special frame structure like a table or a multi-column with column-span. They need to be reframed.
I feel we should reframe the root element when 1 or 2 happens, but not when 3 happens, because 3 can never affect the root element's writing-mode. However, I don't find a simple way to distinguish 1 & 2 from 3 in our current code.
1 and 2 seem hard to detect in our current frame constructor architecture. So maybe we need to add a flag to RecreateFramesForContent and MaybeRecreateContainerForFrameRemoval, and use the flag when the calls are from WipeContainingBlock or WipeInsertionParent to reflect the usecase 3?
Emilio, does the above analysis make sense to you?
|
|
Reporter | |
Comment 8•4 years ago
|
||
We already have flags to differentiate (1) and (2) via RemoveFlags, so that's a possibility and I don't think it'd be hard.
Another possibility is to just don't reframe the root ever, and update the frame tree when writing-mode changes. writing-mode doesn't really affect frame construction except for the BFC bits, right? Something else?
|
|
Assignee | |
Comment 9•4 years ago
•
|
||
OK. I realize we don't need to differentiate (1) and (2).
<html>
<body style="writing-mode: vertical-lr">
<body style="writing-mode: vertical-rl">
</html>
Suppose we have the above snippet. I wrongly assume that after removing the first <body>, we'll need to either update or reframe <html> so that the second body becomes the canonical body and propagates its writing-mode. However the parser only parse the first <body> as the body element, and the second <body> is just a normal block under the first body, so removing the first <body> removes both of them together.
(2) still works if we remove the unconditional reframe root element in MaybeRecreateContainerForFrameRemoval, i.e. never reframe the root element when removing body. Because for whatever reason <body> needs to be reframed in RecreateFramesForContent, it goes to ContentRemoved(..., REMOVE_FOR_RECONSTRUCTION) and then ContentRangeInserted(), which eventually goes to WipeContainingBlock [2], and reframe the root element if its writing-mode is different from the body's writing-mode.
Bottom line: I think we can remove the unconditional reframe root element when the body gets reframed, and everything still works.
[1] https://searchfox.org/mozilla-central/rev/d2cec90777d573585f8477d5170892e5dcdfb0ab/layout/base/nsCSSFrameConstructor.cpp#8601,8618-8619
[2] https://searchfox.org/mozilla-central/rev/d2cec90777d573585f8477d5170892e5dcdfb0ab/layout/base/nsCSSFrameConstructor.cpp#11220-11224
|
|
Assignee | |
Comment 10•4 years ago
|
||
For whatever reasons, we pass body element into
RecreateFramesForContent, the root element won't be reframed down in
the ContentRemoved path, but in ContentRangeInserted() path while
checking WipeContainingBlock. And in bug 1593752, we already have the
logic to reframe root element only when html and body's writing-modes
are different, so we won't over-eagerly reframe the root element.
|
||
Updated•4 years ago
|
|
||
Comment 11•4 years ago
|
||
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/06366bae5966 Remove the logic to reframe root unconditionally when reframing body. r=emilio
|
|
Reporter | |
Comment 12•4 years ago
|
||
Ah, wasn't this logic needed for when we actually have two bodies and we remove the first one?
Arguably an edge case that we didn't use to handle right before anyway (and no other browser seems to), but I think that is why we had this in the first place.
Oh well. We have tests for this now so if we wanted to fix that we'd need to check whether there's actually another body element. Or something.
|
|
Assignee | |
Comment 13•4 years ago
•
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)
Ah, wasn't this logic needed for when we actually have two bodies and we remove the first one?
I don't feel the logic is needed when there are two bodies and the first one is removed, per my explanation in the Suppose we have the above snippet. ... paragraph in comment 9.
For the case when a new body is inserted, the old body currently doesn't get reframed (unless the new body has a different writing-mode from the root element's). This is bug 1594297.
|
||
Comment 14•4 years ago
|
||
| bugherder | ||
|
||
Updated•4 years ago
|
|
|
||
Comment 15•4 years ago
|
||
The patch landed in nightly and beta is affected.
:TYLin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.
For more information, please visit auto_nag documentation.
|
|
Assignee | |
Comment 16•4 years ago
|
||
This bug is not common, which requires a dynamic modification to the <body> and specific script logic. I feel we can have it ride the train.
|
||
Updated•4 years ago
|
Description
•