After removing inline-size from style.contain, the element should trigger layout
Created attachment 457869 [details] Patch
Created attachment 457871 [details] Patch
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
Created attachment 457876 [details] Patch
Comment on attachment 457876 [details] Patch Hi, I think this patch is ready for review. Please take a look, thanks!
Comment on attachment 457876 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457876&action=review > Source/WebCore/rendering/style/RenderStyle.cpp:880 > + if (containsSize() != other.containsSize() || containsInlineSize() != other.containsInlineSize()) Should these checks not be done outside of this if block since it also depends on container queries?
Comment on attachment 457876 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457876&action=review >> Source/WebCore/rendering/style/RenderStyle.cpp:880 >> + if (containsSize() != other.containsSize() || containsInlineSize() != other.containsInlineSize()) > > Should these checks not be done outside of this if block since it also depends on container queries? Hmmm, containerType is inside m_rareNonInheritedData, if they are same, looks like there is no chance that containsInlineSize() is different.
Comment on attachment 457876 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457876&action=review >>> Source/WebCore/rendering/style/RenderStyle.cpp:880 >>> + if (containsSize() != other.containsSize() || containsInlineSize() != other.containsInlineSize()) >> >> Should these checks not be done outside of this if block since it also depends on container queries? > > Hmmm, containerType is inside m_rareNonInheritedData, if they are same, looks like there is no chance that containsInlineSize() is different. Ah, I missed that. How about moving the body of effectiveContainment into StyleRareNonInheritedData and then rareNonInheritedDataChangeRequiresLayout can check containsSize/containsInlineSize? The benefit is that the part you changed above remains easy to read.
Comment on attachment 457876 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457876&action=review >>>> Source/WebCore/rendering/style/RenderStyle.cpp:880 >>>> + if (containsSize() != other.containsSize() || containsInlineSize() != other.containsInlineSize()) >>> >>> Should these checks not be done outside of this if block since it also depends on container queries? >> >> Hmmm, containerType is inside m_rareNonInheritedData, if they are same, looks like there is no chance that containsInlineSize() is different. > > Ah, I missed that. How about moving the body of effectiveContainment into StyleRareNonInheritedData and then rareNonInheritedDataChangeRequiresLayout can check containsSize/containsInlineSize? The benefit is that the part you changed above remains easy to read. Done
Created attachment 458141 [details] Patch
Comment on attachment 458141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458141&action=review > Source/WebCore/ChangeLog:3 > + The layout is not updated when style.contain is changed from "size"/"inline-size"to empty string Extra space after "size"/"inline-size" > Source/WebCore/ChangeLog:8 > + As contain: size/inline-size changes the layout result, but the values "layout", "paint" and Can you rephrase the first sentence? > Source/WebCore/ChangeLog:9 > + "style" are not. So we need to require layout when the value is changed from "size"/"inline-size" s/So we need to require layout/So we require layout > LayoutTests/imported/w3c/web-platform-tests/css/css-contain/contain-inline-size-removed.html:13 > +<div id="container" style="contain: inline-size; width:fit-content; overflow: hidden;"><span>PASS</span></div> Why is width:fit-content needed here and not in the other test case?
Comment on attachment 458141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458141&action=review >> Source/WebCore/ChangeLog:3 >> + The layout is not updated when style.contain is changed from "size"/"inline-size"to empty string > > Extra space after "size"/"inline-size" Done >> Source/WebCore/ChangeLog:8 >> + As contain: size/inline-size changes the layout result, but the values "layout", "paint" and > > Can you rephrase the first sentence? Done >> Source/WebCore/ChangeLog:9 >> + "style" are not. So we need to require layout when the value is changed from "size"/"inline-size" > > s/So we need to require layout/So we require layout Done >> LayoutTests/imported/w3c/web-platform-tests/css/css-contain/contain-inline-size-removed.html:13 >> +<div id="container" style="contain: inline-size; width:fit-content; overflow: hidden;"><span>PASS</span></div> > > Why is width:fit-content needed here and not in the other test case? Because inline-size containment only affects the inline-axis sizing. If there is no "width:fit-content", the widths are same (its container's width). We can't tell if contain:inline-size is applied or not. https://www.w3.org/TR/css-contain-3/#containment-inline-size
Created attachment 458145 [details] Patch
Created attachment 458146 [details] Patch
Committed r293288 (249913@main): <https://commits.webkit.org/249913@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458146 [details].
<rdar://problem/92207316>