210478 2020-04-13 22:27:06 -0700 [css-flexbox] percent children don't resolve against the flex basis on a fully inflexible item with fixed flex-basis 2021-04-27 03:16:20 -0700 1 1 1 Unclassified WebKit CSS WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=210477 InRadar P2 Normal --- 1 clopez svillar changseok darin esprehn+autocc ews-watchlist glenn jfernandez koivisto kondapallykalyan pdr rbuis rego svillar webkit-bug-importer zalan oldest_to_newest 1641254 0 clopez 2020-04-13 22:27:06 -0700 In a column flexbox, the height of a fully inflexible item with fixed flex-basis is definite, and percent children resolve against the flex basis, not the specified height. See: https://drafts.csswg.org/css-flexbox-1/#definite-sizes https://drafts.csswg.org/css-flexbox/#change-2016-inflexible-definite This is tested by WPT test http://wpt.live/css/css-flexbox/percentage-heights-006.html which fails on WebKit 1752787 1 426701 svillar 2021-04-21 09:43:34 -0700 Created attachment 426701 Patch 1754059 2 426701 darin 2021-04-25 13:35:49 -0700 Comment on attachment 426701 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426701&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:1244 > +// This method is only called whenever a descendant of a flex item wants to resolve a percentage in its I’m not a huge fan of the word "method" to mean "member function". Can we just say "function" instead? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1255 > + if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child))) Is this really needed only for the values of "exactly zero"? Are other very small values covered sufficiently by test cases? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1261 > - auto updateDescendants = m_inLayout ? UpdatePercentageHeightDescendants::Yes : UpdatePercentageHeightDescendants::No; > - if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), updateDescendants)) > + if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), UpdatePercentageHeightDescendants::Yes)) What makes the m_inLayout check no longer necessary? 1754126 3 426701 svillar 2021-04-26 01:18:02 -0700 Comment on attachment 426701 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426701&action=review >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1244 >> +// This method is only called whenever a descendant of a flex item wants to resolve a percentage in its > > I’m not a huge fan of the word "method" to mean "member function". Can we just say "function" instead? Sure >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1255 >> + if (child.style().flexGrow() == 0.0 && child.style().flexShrink() == 0.0 && childMainSizeIsDefinite(child, flexBasisForChild(child))) > > Is this really needed only for the values of "exactly zero"? Are other very small values covered sufficiently by test cases? Yes the specs define "inflexible item" as an item whose flex-growth and flex-shrink are zero https://drafts.csswg.org/css-flexbox/#fully-inflexible The main doubt I had was whether to express this as !child.style().flexGrow() or as in the patch. Being a float I decided to use the other syntax but not strong opinion here. Regarding testing, using arbitrary low values are in practice the same as using arbitrary big ones in the sense that we'd end up distributing big chunks of space. There is one test checking that a huge value for shrink/grow does not crash, but apart from that I haven't seen others. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1261 >> + if (!canComputePercentageFlexBasis(child, Length(0, LengthType::Percent), UpdatePercentageHeightDescendants::Yes)) > > What makes the m_inLayout check no longer necessary? Because it was actually wrong. We do always want to update the hasPercentageHeightDescendants flag whenever this function is called. It used to work because in most of the cases this is called as part of layout. 1754540 4 svillar 2021-04-27 03:15:12 -0700 Committed r276634 (237062@main): <https://commits.webkit.org/237062@main> 1754541 5 webkit-bug-importer 2021-04-27 03:16:20 -0700 <rdar://problem/77201507> 426701 2021-04-21 09:43:34 -0700 2021-04-26 11:06:09 -0700 Patch bug-210478-20210421184333.patch text/plain 6145 svillar U3VidmVyc2lvbiBSZXZpc2lvbjogMjc2MzY3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzRlOGYzNzBiYjY1MTcy ZjdmNTNkMDAwYzExZmEwNWE1NDViZGM1Ny4uYmYyZWIzZDNhZmQwMTZlODU1YTBlNDgyNGRiZjFl MDQ0YTllZjM4ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDIxLTA0LTIxICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCisgICAgICAgIFtjc3MtZmxl eGJveF0gcGVyY2VudCBjaGlsZHJlbiBkb24ndCByZXNvbHZlIGFnYWluc3QgdGhlIGZsZXggYmFz aXMgb24gYSBmdWxseSBpbmZsZXhpYmxlIGl0ZW0gd2l0aCBmaXhlZCBmbGV4LWJhc2lzCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTA0NzgKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGbGV4Ym94IHNwZWMg ZGVmaW5lcyBzb21lIGFkZGl0aW9uYWwgY2FzZXMgd2hlcmUgc2l6ZXMgY291bGQgYmUgY29uc2lk ZXJlZCBkZWZpbml0ZSBkZXNwaXRlIHRoZXkgYXJlCisgICAgICAgIGFjdHVhbGx5IGluZGVmaW5p dGUuIE9uZSBvZiB0aG9zZSBjYXNlcyBpcyB3aGVuZXZlciB3ZSBoYXZlIGEgZnVsbHkgaW5mbGV4 aWJsZSBpdGVtIHdpdGggYSBkZWZpbml0ZQorICAgICAgICBmbGV4LWJhc2lzLiBJbiB0aGF0IGNh c2Ugd2Ugc2hvdWxkIGFsbG93IGRlc2RlbmRhbnRzIG9mIGZsZXggaXRlbSB0byByZXNvbHZlIGFn YWluc3QgdGhlIHBvc3QtZmxleGluZworICAgICAgICBzaXplIG9mIHRoZSBmbGV4IGl0ZW0gd2hp Y2ggaXMgYmFzaWNhbGx5IHRoZSB2YWx1ZSBvZiB0aGUgZGVmaW5pdGUgZmxleC1iYXNpcy4gTm90 ZSB0aGF0IGluIHRoZSBtYWpvcml0eQorICAgICAgICBvZiBjYXNlcyB0aG9zZSBwZXJjZW50YWdl cyBhcmUgcmVzb2x2ZWQgYmVmb3JlIGZsZXhpbmcgdGhlIGl0ZW0gYW5kIG5vdCByZWNvbXB1dGVk IGFmdGVyIHRoYXQuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyRmxleGlibGVCb3guY3Bw OgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyRmxleGlibGVCb3g6OnVzZUNoaWxkT3ZlcnJpZGlu Z01haW5TaXplRm9yUGVyY2VudGFnZVJlc29sdXRpb24pOiBsZXQgZGVzY2VuZGFudHMgb2YKKyAg ICAgICAgZnVsbHkgaW5mbGV4aWJsZSBmbGV4IGl0ZW1zIHdpdGggZGVmaW5pdGUgZmxleC1iYXNp cyB0byByZXNvbHZlIGFnYWluc3QgdGhlIHBvc3QtZmxleGluZyBzaXplLiBBbHNvIGFkZGVkCisg ICAgICAgIHNvbWUgZG9jdW1lbnRhdGlvbiBiZWNhdXNlIHRoYXQgcGFydCBvZiB0aGUgY29kZSBp cyBmYXIgZnJvbSBvYnZpb3VzIGZvciB0aGUgY2FzdWFsIHJlYWRlci4KKwogMjAyMS0wNC0yMSAg UGVuZyBMaXUgIDxwZW5nLmxpdTZAYXBwbGUuY29tPgogCiAgICAgICAgIFtHUFVQXSBSZWZhY3Rv ciB0aGUgaW1wbGVtZW50YXRpb24gb2YgTWVkaWFTb3VyY2U6OmJ1ZmZlcmVkKCkKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJGbGV4aWJsZUJveC5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyRmxleGlibGVCb3guY3BwCmluZGV4IGRiODMyYTVi MmVmMGU1NGRjMTI4ZTVhMTc1YjY5MTEwM2MwZmE2OGQuLjc3YWQ1Zjk5OGYxZGJjYjk0YWJlODE1 MWFlNThiZjczMGRjNzk4NDUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9S ZW5kZXJGbGV4aWJsZUJveC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ckZsZXhpYmxlQm94LmNwcApAQCAtMTI0MSwxNCArMTI0MSwyNCBAQCBib29sIFJlbmRlckZsZXhp YmxlQm94Ojp1c2VDaGlsZE92ZXJyaWRpbmdDcm9zc1NpemVGb3JQZXJjZW50YWdlUmVzb2x1dGlv bihjb25zdAogICAgIHJldHVybiBjaGlsZC5oYXNPdmVycmlkaW5nTG9naWNhbEhlaWdodCgpOwog fQogCisvLyBUaGlzIG1ldGhvZCBpcyBvbmx5IGNhbGxlZCB3aGVuZXZlciBhIGRlc2NlbmRhbnQg b2YgYSBmbGV4IGl0ZW0gd2FudHMgdG8gcmVzb2x2ZSBhIHBlcmNlbnRhZ2UgaW4gaXRzCisvLyBi bG9jayBheGlzIChsb2dpY2FsIGhlaWdodCkuIFRoZSBrZXkgaGVyZSBpcyB0aGF0IHBlcmNlbnRh Z2VzIHNob3VsZCBiZSBnZW5lcmFsbHkgcmVzb2x2ZWQgYmVmb3JlIHRoZQorLy8gZmxleCBpdGVt IGlzIGZsZXhlZCwgbWVhbmluZyB0aGF0IHRoZXkgc2hvdWxkbid0IGJlIHJlY29tcHV0ZWQgb25j ZSB0aGUgZmxleCBpdGVtIGhhcyBiZWVuIGZsZXhlZC4gVGhlcmUKKy8vIGFyZSBzb21lIGV4Y2Vw dGlvbnMgdGhvdWdoIHRoYXQgYXJlIGltcGxlbWVudGVkIGhlcmUsIGxpa2UgdGhlIGNhc2Ugb2Yg ZnVsbHkgaW5mbGV4aWJsZSBpdGVtcyB3aXRoCisvLyBkZWZpbml0ZSBmbGV4LWJhc2lzLCBvciB3 aGVuZXZlciB0aGUgZmxleCBjb250YWluZXIgaGFzIGEgZGVmaW5pdGUgbWFpbiBzaXplLiBTZWUK Ky8vIGh0dHBzOi8vZHJhZnRzLmNzc3dnLm9yZy9jc3MtZmxleGJveC8jZGVmaW5pdGUtc2l6ZXMg Zm9yIGFkZGl0aW9uYWwgZGV0YWlscy4KIGJvb2wgUmVuZGVyRmxleGlibGVCb3g6OnVzZUNoaWxk T3ZlcnJpZGluZ01haW5TaXplRm9yUGVyY2VudGFnZVJlc29sdXRpb24oY29uc3QgUmVuZGVyQm94 JiBjaGlsZCkKIHsKICAgICBBU1NFUlQoIW1haW5BeGlzSXNDaGlsZElubGluZUF4aXMoY2hpbGQp KTsKKworICAgIC8vIFRoZSBtYWluIHNpemUgb2YgYSBmdWxseSBpbmZsZXhpYmxlIGl0ZW0gd2l0 aCBhIGRlZmluaXRlIGZsZXggYmFzaXMgaXMsIGJ5IGRlZmluaXRpb24sIGRlZmluaXRlLgorICAg IGlmIChjaGlsZC5zdHlsZSgpLmZsZXhHcm93KCkgPT0gMC4wICYmIGNoaWxkLnN0eWxlKCkuZmxl eFNocmluaygpID09IDAuMCAmJiBjaGlsZE1haW5TaXplSXNEZWZpbml0ZShjaGlsZCwgZmxleEJh c2lzRm9yQ2hpbGQoY2hpbGQpKSkKKyAgICAgICAgcmV0dXJuIGNoaWxkLmhhc092ZXJyaWRpbmdM b2dpY2FsSGVpZ2h0KCk7CisKICAgICAvLyBUaGlzIGZ1bmN0aW9uIGltcGxlbWVudHMgc2VjdGlv biA5LjguIERlZmluaXRlIGFuZCBJbmRlZmluaXRlIFNpemVzLCBjYXNlIDIpIG9mIHRoZSBmbGV4 Ym94IHNwZWMuCiAgICAgLy8gSWYgdGhlIGZsZXggY29udGFpbmVyIGhhcyBhIGRlZmluaXRlIG1h aW4gc2l6ZSB0aGUgZmxleCBpdGVtIHBvc3QtZmxleGluZyBtYWluIHNpemUgaXMgYWxzbyB0cmVh dGVkCiAgICAgLy8gYXMgZGVmaW5pdGUuIFdlIG1ha2UgdXAgYSBwZXJjZW50YWdlIHRvIGNoZWNr IHdoZXRoZXIgd2UgaGF2ZSBhIGRlZmluaXRlIHNpemUuCi0gICAgYXV0byB1cGRhdGVEZXNjZW5k YW50cyA9IG1faW5MYXlvdXQgPyBVcGRhdGVQZXJjZW50YWdlSGVpZ2h0RGVzY2VuZGFudHM6Olll cyA6IFVwZGF0ZVBlcmNlbnRhZ2VIZWlnaHREZXNjZW5kYW50czo6Tm87Ci0gICAgaWYgKCFjYW5D b21wdXRlUGVyY2VudGFnZUZsZXhCYXNpcyhjaGlsZCwgTGVuZ3RoKDAsIExlbmd0aFR5cGU6OlBl cmNlbnQpLCB1cGRhdGVEZXNjZW5kYW50cykpCisgICAgaWYgKCFjYW5Db21wdXRlUGVyY2VudGFn ZUZsZXhCYXNpcyhjaGlsZCwgTGVuZ3RoKDAsIExlbmd0aFR5cGU6OlBlcmNlbnQpLCBVcGRhdGVQ ZXJjZW50YWdlSGVpZ2h0RGVzY2VuZGFudHM6OlllcykpCiAgICAgICAgIHJldHVybiBmYWxzZTsK IAogICAgIHJldHVybiBjaGlsZC5oYXNPdmVycmlkaW5nTG9naWNhbEhlaWdodCgpOwpkaWZmIC0t Z2l0IGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4 IGQwYTM4ZDYwMmUxYTdlNmNiNmU5MDNjMmZiZjYzYzJiMGYxM2FkMGQuLjkxYzAyZWNhNDI4NTNj NzBmYmNjOTFhMTRhOTkxMzYxNzZlODdlMjYgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5n ZUxvZworKysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMiBAQAorMjAyMS0w NC0yMSAgU2VyZ2lvIFZpbGxhciBTZW5pbiAgPHN2aWxsYXJAaWdhbGlhLmNvbT4KKworICAgICAg ICBbY3NzLWZsZXhib3hdIHBlcmNlbnQgY2hpbGRyZW4gZG9uJ3QgcmVzb2x2ZSBhZ2FpbnN0IHRo ZSBmbGV4IGJhc2lzIG9uIGEgZnVsbHkgaW5mbGV4aWJsZSBpdGVtIHdpdGggZml4ZWQgZmxleC1i YXNpcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEw NDc4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBU ZXN0RXhwZWN0YXRpb25zOiBVbnNraXBwZWQgYSBjb3VwbGUgb2YgdGVzdHMgdGhhdCBhcmUgcGFz c2luZyBub3cuCisKIDIwMjEtMDQtMjEgIEFkaXR5YSBLZWVydGhpICA8YWtlZXJ0aGlAYXBwbGUu Y29tPgogCiAgICAgICAgIFtpT1NdW0ZDUl0gPHNlbGVjdD4gZGVjb3JhdGlvbnMgc2hvdWxkIG1h dGNoIHRoZSB0ZXh0IGNvbG9yCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9UZXN0RXhwZWN0YXRp b25zIGIvTGF5b3V0VGVzdHMvVGVzdEV4cGVjdGF0aW9ucwppbmRleCA2ZTkyMzI3NjBlYjExNTdk OWYxNjYyN2JjMTA0NmQ0M2NjYTYyN2VlLi4wODIwMjAzMzNkZTg1NmQzOTljMTljNmU5NzM3YTkx OTE5NmNhOTE0IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9UZXN0RXhwZWN0YXRpb25zCisrKyBi L0xheW91dFRlc3RzL1Rlc3RFeHBlY3RhdGlvbnMKQEAgLTM5MDksMTEgKzM5MDksOSBAQCB3ZWJr aXQub3JnL2IvMjE5MzQzIGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1m bGV4Ym94L2ZsZXgtYXNwZWN0LQogd2Via2l0Lm9yZy9iLzE0NTE3NiBpbXBvcnRlZC93M2Mvd2Vi LXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3MtZmxleGJveC9mbGV4Ym94X2FsaWduLWl0ZW1zLXN0cmV0 Y2gtMy5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCiB3ZWJraXQub3JnL2IvMjEwMDkzIGltcG9y dGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1mbGV4Ym94L3NlbGVjdC1lbGVtZW50 LXplcm8taGVpZ2h0LTAwMi5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCiB3ZWJraXQub3JnL2Iv MjEwMTQ0IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1mbGV4Ym94L2Fu b255bW91cy1mbGV4LWl0ZW0tMDA1Lmh0bWwgWyBJbWFnZU9ubHlGYWlsdXJlIF0KLXdlYmtpdC5v cmcvYi8yMTA0NzggaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3NzLWZsZXhi b3gvcGVyY2VudGFnZS1oZWlnaHRzLTAwNi5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCiB3ZWJr aXQub3JnL2IvMjEyMDQ2IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1m bGV4Ym94L292ZXJmbG93LWFyZWEtMDAxLmh0bWwgWyBJbWFnZU9ubHlGYWlsdXJlIF0KIHdlYmtp dC5vcmcvYi8yMTIwNDYgaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3NzLWZs ZXhib3gvb3ZlcmZsb3ctYXJlYS0wMDIuaHRtbCBbIEltYWdlT25seUZhaWx1cmUgXQogd2Via2l0 Lm9yZy9iLzIxMjA0NiBpbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3MtZmxl eGJveC9vdmVyZmxvdy1hdXRvLTAwNS5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCi13ZWJraXQu b3JnL2IvMjEyMDQ2IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1mbGV4 Ym94L3BlcmNlbnRhZ2UtaGVpZ2h0cy0wMDcuaHRtbCBbIEltYWdlT25seUZhaWx1cmUgXQogd2Vi a2l0Lm9yZy9iLzIxMjA0NiBpbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3Mt ZmxleGJveC9wZXJjZW50YWdlLWhlaWdodHMtMDE0Lmh0bWwgWyBJbWFnZU9ubHlGYWlsdXJlIF0K IHdlYmtpdC5vcmcvYi8yMTIwNDYgaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3Mv Y3NzLWZsZXhib3gvc2Nyb2xsYmFycy1hdXRvLmh0bWwgWyBJbWFnZU9ubHlGYWlsdXJlIF0KIHdl YmtpdC5vcmcvYi8yMTIwNDYgaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3Nz LWZsZXhib3gvc2Nyb2xsYmFycy5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCg==