Bug 225489

Summary:

[css-flexbox] Flex item construction may affect sibling flex item height computation

Product:

WebKit

Reporter:

zalan <zalan>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cgarcia, changseok, clopez, commit-queue, esprehn+autocc, ews-watchlist, glenn, gsnedders, kondapallykalyan, pdr, simon.fraser, svillar, youennf, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

225618

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
[fast-cq] Patch	none
Test reduction for the crash after the fix	none
Patch	none

zalan

Reported 2021-05-06 15:37:48 PDT

e.g. <!DOCTYPE html> <div style="height: 100px; position: relative; display: flex; flex-direction: column;"> <div style="background-color: yellow; height: 50px;"><div style="height: 100%; background-color: green"></div></div> <div style="height: 50%; background-color: green"></div> </div> vs. <!DOCTYPE html> <div style="height: 100px; position: relative; display: flex; flex-direction: column;"> <div style="height: 50%; background-color: green"></div> <div style="background-color: yellow; height: 50px;"><div style="height: 100%; background-color: green"></div></div> </div> these cases should match. <rdar://75050281>

Attachments
Patch (5.81 KB, patch) 2021-05-06 17:20 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (14.37 KB, patch) 2021-05-07 09:11 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (15.07 KB, patch) 2021-05-07 16:22 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (15.07 KB, patch) 2021-05-07 16:23 PDT, zalan	no flags	Details Formatted Diff Diff
[fast-cq] Patch (14.38 KB, patch) 2021-05-07 18:46 PDT, zalan	no flags	Details Formatted Diff Diff
Test reduction for the crash after the fix (399 bytes, text/html) 2021-05-11 09:10 PDT, zalan	no flags	Details
Patch (7.17 KB, patch) 2021-05-11 11:22 PDT, zalan	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

zalan

Comment 1 2021-05-06 17:20:28 PDT

Created attachment 427962 [details] Patch

Sergio Villar Senin

Comment 2 2021-05-07 02:20:39 PDT

Comment on attachment 427962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427962&action=review Good catch! I think we can fix it in a slightly better way though. The actual issue IMO is that we're updating m_hasDefiniteHeight when we should not. That is controlled by the infamous m_inLayout bool which has been quite confusing (I added some bugs due to it in the past) and with a superwide scope. My proposal to fix this issue would be: 1- Remove the m_inLayout handling from ::layoutBlock() 2- Rename m_inLayout to m_inFlexItemsConstruction for example (should be only used in canComputePercentageFlexBasis()) 3- Wrap the loop that creates the flex items with a pair of braces to define a new scope and something like: SetForScope<bool> inFlexItemConstruction(m_inFlexItemsConstruction, true); 4- Remove the m_hasDefiniteHeight = SizeDefiniteness::Unknown; from ::layoutFlexItems as it won't be needed (it's properly reset by the end of layoutBlock()) Does it make sense? > LayoutTests/ChangeLog:9 > + * fast/flexbox/flex-column-with-percent-height-descendants.html: Added. Nice test, WDYT about uploading it to the WPT directory instead? I think it'd serve well to all the other engines.

zalan

Comment 3 2021-05-07 07:26:05 PDT

(In reply to Sergio Villar Senin from comment #2) > Comment on attachment 427962 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=427962&action=review > > Good catch! > > I think we can fix it in a slightly better way though. The actual issue IMO > is that we're updating m_hasDefiniteHeight when we should not. Right, that's why the reset hack is right here after the constructFlexItem call (and the comment trying to explain it). In this patch I was just fixing the hack. I was thinking of removing it but since this part of the code has been the source of regressions lately, I decided not to. But you are correct, we should do that and in case of a new regression we could always fall back to this version. The _actual_actual_ issue here is the way we handle the percent height resolution (gOverridingContainingBlockContentLogicalHeightMap) and that we have to scope certain calls to _not_ update certain states indirectly. This is due to the fact that in rendering we call the containing block chain for constraints (in LFC, the constrains are pushed down to the geometry functions, so whenever we try to resolve some property (width/height etc) the constraints are readily available). > controlled by the infamous m_inLayout bool which has been quite confusing (I > added some bugs due to it in the past) and with a superwide scope. My > proposal to fix this issue would be: > 1- Remove the m_inLayout handling from ::layoutBlock() > 2- Rename m_inLayout to m_inFlexItemsConstruction for example (should be > only used in canComputePercentageFlexBasis()) > 3- Wrap the loop that creates the flex items with a pair of braces to define > a new scope and something like: > SetForScope<bool> inFlexItemConstruction(m_inFlexItemsConstruction, true); > 4- Remove the m_hasDefiniteHeight = SizeDefiniteness::Unknown; from > ::layoutFlexItems as it won't be needed (it's properly reset by the end of > layoutBlock()) > > Does it make sense? Yeah, I am happy to remove m_inLayout. It does not do any good. Thanks for the review.

zalan

Comment 4 2021-05-07 09:11:55 PDT

Created attachment 428002 [details] Patch

Sergio Villar Senin

Comment 5 2021-05-07 13:00:43 PDT

Comment on attachment 428002 [details] Patch Nice!. Still think the test makes sense in WPT.

Sergio Villar Senin

Comment 6 2021-05-07 13:06:51 PDT

(In reply to zalan from comment #3) > > I think we can fix it in a slightly better way though. The actual issue IMO > > is that we're updating m_hasDefiniteHeight when we should not. > Right, that's why the reset hack is right here after the constructFlexItem > call (and the comment trying to explain it). > In this patch I was just fixing the hack. I was thinking of removing it but > since this part of the code has been the source of regressions lately, I > decided not to. > But you are correct, we should do that and in case of a new regression we > could always fall back to this version. > The _actual_actual_ issue here is the way we handle the percent height > resolution (gOverridingContainingBlockContentLogicalHeightMap) and that we > have to scope certain calls to _not_ update certain states indirectly. This > is due to the fact that in rendering we call the containing block chain for > constraints (in LFC, the constrains are pushed down to the geometry > functions, so whenever we try to resolve some property (width/height etc) > the constraints are readily available). That's nice, I guess it'd make the computation of definite/indefinite sizes much easier. And if those constrains could be set at will that would be a massive win for layout systems like grid or flex, where the container sets specific constrains (the overriding sizes).

zalan

Comment 7 2021-05-07 13:47:47 PDT

(In reply to Sergio Villar Senin from comment #5) > Comment on attachment 428002 [details] > Patch > > Nice!. Still think the test makes sense in WPT. Oh right. I totally forgot about that. Thanks for reminding me (and thanks for the review)

zalan

Comment 8 2021-05-07 16:22:00 PDT

Created attachment 428055 [details] Patch

zalan

Comment 9 2021-05-07 16:23:00 PDT

Created attachment 428056 [details] Patch

EWS Watchlist

Comment 10 2021-05-07 16:23:19 PDT

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

zalan

Comment 11 2021-05-07 16:24:38 PDT

Sam, could you help me figure out the process here (re: WPT)?

zalan

Comment 12 2021-05-07 16:25:06 PDT

(In reply to EWS Watchlist from comment #10) > This patch modifies the imported WPT tests. Please ensure that any changes > on the tests (not coming from a WPT import) are exported to WPT. Please see > https://trac.webkit.org/wiki/WPTExportProcess oh there it is.

zalan

Comment 13 2021-05-07 16:28:58 PDT

(In reply to zalan from comment #12) > (In reply to EWS Watchlist from comment #10) > > This patch modifies the imported WPT tests. Please ensure that any changes > > on the tests (not coming from a WPT import) are exported to WPT. Please see > > https://trac.webkit.org/wiki/WPTExportProcess > oh there it is. > 6. Land the PR in WPT > 7. Finally land the patch in WebKit 6 needs to happen first?

zalan

Comment 14 2021-05-07 16:32:22 PDT

alright, I am going to land the original patch. will deal with WPT later.

EWS

Comment 15 2021-05-07 16:33:47 PDT

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Sergio Villar Senin

Comment 16 2021-05-07 16:37:06 PDT

(In reply to zalan from comment #14) > alright, I am going to land the original patch. will deal with WPT later. The export to WPT is unreliable in my experience. What I normally do is to create a PR in WPT and after it's merged then I import the tests in WebKit using import-w3c-tests. It's a bit clumsy but it works.

zalan

Comment 17 2021-05-07 18:46:50 PDT

Created attachment 428069 [details] [fast-cq] Patch

zalan

Comment 18 2021-05-07 18:47:41 PDT

(In reply to Sergio Villar Senin from comment #16) > (In reply to zalan from comment #14) > > alright, I am going to land the original patch. will deal with WPT later. > > The export to WPT is unreliable in my experience. What I normally do is to > create a PR in WPT and after it's merged then I import the tests in WebKit > using import-w3c-tests. It's a bit clumsy but it works. Thanks for the heads up. Will create a PR for this. and thanks for the review again.

EWS

Comment 19 2021-05-07 18:48:32 PDT

Committed r277222 (237493@main): <https://commits.webkit.org/237493@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428069 [details].

Carlos Garcia Campos

Comment 20 2021-05-10 03:05:30 PDT

I'm getting a crash after this change in matrix web application with epiphany. It's a release assert, see the bt: #0 0x00007f580845cc2e in WTFCrash () from /home/cgarcia/gnome/lib/libjavascriptcoregtk-4.1.so.0 #1 0x00007f580bcb7305 in WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild(WebCore::RenderBox&, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #2 0x00007f580bcc6463 in WebCore::RenderFlexibleBox::constructFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #3 0x00007f580bcc7729 in WebCore::RenderFlexibleBox::layoutFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #4 0x00007f580bcc8067 in WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #5 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #6 0x00007f580bcc372f in WebCore::RenderFlexibleBox::layoutAndPlaceChildren(WebCore::LayoutUnit&, WTF::Vector<WebCore::FlexItem, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit, bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #7 0x00007f580bcc79ba in WebCore::RenderFlexibleBox::layoutFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #8 0x00007f580bcc8067 in WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #9 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #10 0x00007f580bcc372f in WebCore::RenderFlexibleBox::layoutAndPlaceChildren(WebCore::LayoutUnit&, WTF::Vector<WebCore::FlexItem, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit, bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #11 0x00007f580bcc79ba in WebCore::RenderFlexibleBox::layoutFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #12 0x00007f580bcc8067 in WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #13 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #14 0x00007f580bcc372f in WebCore::RenderFlexibleBox::layoutAndPlaceChildren(WebCore::LayoutUnit&, WTF::Vector<WebCore::FlexItem, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit, bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #15 0x00007f580bcc79ba in WebCore::RenderFlexibleBox::layoutFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #16 0x00007f580bcc8067 in WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #17 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #18 0x00007f580bcc372f in WebCore::RenderFlexibleBox::layoutAndPlaceChildren(WebCore::LayoutUnit&, WTF::Vector<WebCore::FlexItem, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit, bool, WTF::Vector<WebCore::RenderFlexibleBox::LineContext, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #19 0x00007f580bcc79ba in WebCore::RenderFlexibleBox::layoutFlexItems(bool) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #20 0x00007f580bcc8067 in WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #21 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #22 0x00007f580bc42b2f in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #23 0x00007f580bc467dd in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #24 0x00007f580bc4979d in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #25 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #26 0x00007f580bc42b2f in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #27 0x00007f580bc467dd in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #28 0x00007f580bc4979d in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #29 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #30 0x00007f580bc42b2f in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #31 0x00007f580bc467dd in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #32 0x00007f580bc4979d in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #33 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #34 0x00007f580bc42b2f in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 --Type <RET> for more, q to quit, c to continue without paging-- #35 0x00007f580bc467dd in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #36 0x00007f580bc4979d in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #37 0x00007f580bc201ea in WebCore::RenderBlock::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #38 0x00007f580bdff371 in WebCore::RenderView::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #39 0x00007f580b87648f in WebCore::FrameViewLayoutContext::layout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #40 0x00007f580b197082 in WebCore::Document::updateLayout() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #41 0x00007f580b197892 in WebCore::Document::updateLayoutIgnorePendingStylesheets(WebCore::Document::RunPostLayoutTasks) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #42 0x00007f580b1b38b8 in WebCore::Element::scrollTop() () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #43 0x00007f580a368611 in WebCore::jsElement_scrollTop(JSC::JSGlobalObject*, long, JSC::PropertyName) () from /home/cgarcia/gnome/lib/libwebkit2gtk-4.1.so.0 #44 0x00007f580816d07c in JSC::PropertySlot::customGetter(JSC::JSGlobalObject*, JSC::PropertyName) const () from /home/cgarcia/gnome/lib/libjavascriptcoregtk-4.1.so.0 #45 0x00007f5807d96e3b in JSC::LLInt::performLLIntGetByID(JSC::Instruction const*, JSC::CodeBlock*, JSC::JSGlobalObject*, JSC::JSValue, JSC::Identifier const&, JSC::GetByIdModeMetadata&) () from /home/cgarcia/gnome/lib/libjavascriptcoregtk-4.1.so.0 #46 0x00007f5807d97c34 in llint_slow_path_get_by_id () from /home/cgarcia/gnome/lib/libjavascriptcoregtk-4.1.so.0 #47 0x00007f580726842a in llint_op_get_by_id () from /home/cgarcia/gnome/lib/libjavascriptcoregtk-4.1.so.0 #48 0x00007f579020cae0 in ?? ()

zalan

Comment 21 2021-05-10 08:23:06 PDT

It sounds like some code was relying on incorrect state before. Any chance of having the test content/reduction? Thanks.

Sergio Villar Senin

Comment 22 2021-05-10 08:59:51 PDT

(In reply to zalan from comment #21) > It sounds like some code was relying on incorrect state before. Any chance > of having the test content/reduction? Thanks. I'll try to get more data, as it crashes in an internal Igalia url

zalan

Comment 23 2021-05-10 09:11:49 PDT

(In reply to Sergio Villar Senin from comment #22) > (In reply to zalan from comment #21) > > It sounds like some code was relying on incorrect state before. Any chance > > of having the test content/reduction? Thanks. > > I'll try to get more data, as it crashes in an internal Igalia url Thanks. Appreciate it!

Sergio Villar Senin

Comment 24 2021-05-10 11:09:24 PDT

(In reply to zalan from comment #23) > (In reply to Sergio Villar Senin from comment #22) > > (In reply to zalan from comment #21) > > > It sounds like some code was relying on incorrect state before. Any chance > > > of having the test content/reduction? Thanks. > > > > I'll try to get more data, as it crashes in an internal Igalia url > Thanks. Appreciate it! I cannot easily get a test case, but I think I know where the problem comes from. It's basically caused by the m_inLayout removal. canComputePercentageFlexBasis() assumes now (after the patch for this bug) that if we are not computing the flex base size then it's because we're laying out ourselves and thus we should update the m_hasDefiniteHeight value. That's clearly wrong because canComputePercentageFlexBasis() can be called from useChildOverridingMainSizeForPercentageResolution() which can be called at any time by availableLogicalHeightUsing(). Updating the m_hasDefiniteHeight while we are not laying out ourselves could easily lead to false positives. If that happens then the childMainSizeIsDefinite() would return true for flex containers where the main size is the child block axis so when executing if (childMainSizeIsDefinite(child, flexBasis)) return std::max(0_lu, computeMainAxisExtentForChild(child, MainOrPreferredSize, flexBasis).value()); then it'll crash because computeMainAxisExtentForChild() could return nullopt. That must not happen because we have checked that childMainSizeIsDefinite(), but as we have seen, a wrong m_hasDefiniteHeight could deceive us.

WebKit Commit Bot

Comment 25 2021-05-10 14:57:44 PDT

Re-opened since this is blocked by bug 225618

zalan

Comment 26 2021-05-11 09:10:35 PDT

Created attachment 428280 [details] Test reduction for the crash after the fix

zalan

Comment 27 2021-05-11 11:00:23 PDT

so, right, the issue with the patch is that the m_inFlexItemConstruction flag does not exactly cover the !m_inLayout cases and we end up setting the m_hasDefiniteHeight when !m_inFlexItemConstruction but also not in layout. I'll go with my original patch.

zalan

Comment 28 2021-05-11 11:22:25 PDT

Created attachment 428301 [details] Patch

zalan

Comment 29 2021-05-11 11:23:14 PDT

Could you check if you are still seeing the crash with this patch applied, please?

Carlos Garcia Campos

Comment 30 2021-05-13 01:29:44 PDT

(In reply to zalan from comment #29) > Could you check if you are still seeing the crash with this patch applied, > please? No crashes here. Thanks!

zalan

Comment 31 2021-05-13 04:09:33 PDT

Great! Thanks for confirming it!

Sergio Villar Senin

Comment 32 2021-05-13 07:20:03 PDT

Comment on attachment 428301 [details] Patch Let's see if we can eventually manage to get rid of m_inLayout in the future. In the meantime let's make it work correctly at least.

zalan

Comment 33 2021-05-13 07:52:07 PDT

(In reply to Sergio Villar Senin from comment #32) > Comment on attachment 428301 [details] > Patch > > Let's see if we can eventually manage to get rid of m_inLayout in the > future. In the meantime let's make it work correctly at least. Sounds like a plan. :)

EWS

Comment 34 2021-05-13 08:00:20 PDT

Committed r277435 (237683@main): <https://commits.webkit.org/237683@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428301 [details].

Note You need to log in before you can comment on or make changes to this bug.