Bug 249919

Summary: [contain-intrinsic-size] auto-009.html is failing
Product: WebKit Reporter: cathiechen <cathiechen>
Component: CSSAssignee: cathiechen <cathiechen>
Status: RESOLVED FIXED    
Severity: Normal CC: obrufau, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 239450    
Bug Blocks:    
Attachments:
Description Flags
WIP-patch
none
WIP-patch
none
WIP-patch
none
WIP-patch
none
WIP-patch none

Description cathiechen 2022-12-27 21:39:35 PST 
To fix auto-009.html, looks like we need to store last remembered width and height instead of size.
Comment 1 cathiechen 2022-12-27 21:40:02 PST 
Created attachment 464232 [details]
WIP-patch
Comment 2 Radar WebKit Bug Importer 2023-01-03 21:40:17 PST 
<rdar://problem/103860290>
Comment 3 Oriol Brufau 2023-01-09 06:58:54 PST 
Comment on attachment 464232 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464232&action=review

> Source/WebCore/dom/ElementRareData.cpp:44
> +    LayoutUnit lastRemembedHeight;

These should be logical (since ResizeObserver tracks changes to the logical sizes).
Comment 4 cathiechen 2023-02-09 05:32:58 PST 
Created attachment 464921 [details]
WIP-patch
Comment 5 cathiechen 2023-02-09 05:33:29 PST 
(In reply to Oriol Brufau from comment #3)
> Comment on attachment 464232 [details]
> WIP-patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=464232&action=review
> 
> > Source/WebCore/dom/ElementRareData.cpp:44
> > +    LayoutUnit lastRemembedHeight;
> 
> These should be logical (since ResizeObserver tracks changes to the logical
> sizes).

Done, thanks:)
Comment 6 Oriol Brufau 2023-02-09 06:40:12 PST 
Comment on attachment 464921 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464921&action=review

> Source/WebCore/dom/Element.cpp:4484
> +void Element::clearLastRememberedLogicalSize()

Maybe clearLastRememberedSizes? No need to consider it logical since it's clearing both.

> Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:351
> +            element.setLastRememberedLogicalWidth(LayoutUnit(-1));

You are checking the physical containIntrinsicWidthType but clearing the logical setLastRememberedLogicalWidth.
I don't think this will work in a vertical writing mode.
Comment 7 cathiechen 2023-02-09 08:08:15 PST 
Comment on attachment 464921 [details]
WIP-patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464921&action=review

>> Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:351
>> +            element.setLastRememberedLogicalWidth(LayoutUnit(-1));
> 
> You are checking the physical containIntrinsicWidthType but clearing the logical setLastRememberedLogicalWidth.
> I don't think this will work in a vertical writing mode.

Ah, yes, should check writing mode here! Will do, thanks!
Comment 8 cathiechen 2023-02-10 04:17:57 PST 
Created attachment 464940 [details]
WIP-patch
Comment 9 cathiechen 2023-02-10 05:49:56 PST 
Created attachment 464941 [details]
WIP-patch
Comment 10 cathiechen 2023-02-10 06:15:21 PST 
Created attachment 464942 [details]
WIP-patch
Comment 11 cathiechen 2023-02-20 05:34:14 PST 
Pull request: https://github.com/WebKit/WebKit/pull/10359
Comment 12 EWS 2023-02-28 06:17:45 PST 
Committed 260939@main (d8e31024efa9): <https://commits.webkit.org/260939@main>

Reviewed commits have been landed. Closing PR #10359 and removing active labels.