229996 2021-09-07 04:38:43 -0700 [css-flexbox] Add support for self-start & self-end css-align-3 positional alignment properties 2021-09-10 09:14:21 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 svillar svillar changseok clopez esprehn+autocc ews-watchlist glenn jfernandez koivisto kondapallykalyan pdr rego simon.fraser svillar webkit-bug-importer oldest_to_newest 1790956 0 svillar 2021-09-07 04:38:43 -0700 [css-flexbox] Add support for self-start & self-end css-align-3 positional alignment properties 1790963 1 437489 svillar 2021-09-07 05:01:59 -0700 Created attachment 437489 Patch 1790983 2 437489 rego 2021-09-07 06:48:42 -0700 Comment on attachment 437489 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437489&action=review > Source/WebCore/ChangeLog:8 > + Added support for Left and Right positional alignment properties from Left and Right? Isn't now SelfStart and SelfEnd? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1727 > + bool isOrthogonal = style().isHorizontalWritingMode() != child.style().isHorizontalWritingMode(); Nit: We should move that isOrthogonal() method to RenderObject one of these days. O:-) > Source/WebCore/rendering/RenderFlexibleBox.cpp:1728 > + if (isOrthogonal && (style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())) I don't understand this comparison "(style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())". > LayoutTests/TestExpectations:4168 > webkit.org/b/221468 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-align-self-horiz-002.xhtml [ ImageOnlyFailure ] This test is pretty similar to the next one, why is this not passing? 1792289 3 437489 svillar 2021-09-10 05:03:47 -0700 Comment on attachment 437489 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437489&action=review >> Source/WebCore/ChangeLog:8 >> + Added support for Left and Right positional alignment properties from > > Left and Right? Isn't now SelfStart and SelfEnd? Ooops copy-paste is hard :) >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1727 >> + bool isOrthogonal = style().isHorizontalWritingMode() != child.style().isHorizontalWritingMode(); > > Nit: We should move that isOrthogonal() method to RenderObject one of these days. O:-) sure :), let me land this sequence of patches and I'll do it. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1728 >> + if (isOrthogonal && (style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())) > > I don't understand this comparison "(style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())". A bit convoluted maybe. The general idea is that for orthogonal modes SelfStart corresponds to FlexStart with a couple of exceptions, which is when the container has flipped blocks writing mode (vrl or hbt) and the child is LTR, or when it isn't flipped (vlr or htb) but the child is RTL. A couple of examples could illustrate it better: 1) a HTB ltr child inside a VRL container. In this case the physical left edge is flex item's self-start but flexbox line's flex-end 2) a HTB RTL child inside a VLR container. In this case the physical left edge is flex item's self-end but flexbox line's flex-start >> LayoutTests/TestExpectations:4168 >> webkit.org/b/221468 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-align-self-horiz-002.xhtml [ ImageOnlyFailure ] > > This test is pretty similar to the next one, why is this not passing? Interesting and good question, checking the test case, there is just one block that is not positioned correctly. I'll try to figure out why. 1792291 4 svillar 2021-09-10 05:13:21 -0700 (In reply to Sergio Villar Senin from comment #3) > Comment on attachment 437489 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437489&action=review [...] > >> LayoutTests/TestExpectations:4168 > >> webkit.org/b/221468 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-align-self-horiz-002.xhtml [ ImageOnlyFailure ] > > > > This test is pretty similar to the next one, why is this not passing? > > Interesting and good question, checking the test case, there is just one > block that is not positioned correctly. I'll try to figure out why. It turns out that the test result is correct, it's the expectation the one that is not properly rendered. I'll research why but IMHO it shouldn't block this patch as it is a different bug (the expectations are not using the alignment properties at all). 1792322 5 437489 rego 2021-09-10 07:02:08 -0700 Comment on attachment 437489 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437489&action=review r=me >>> Source/WebCore/rendering/RenderFlexibleBox.cpp:1728 >>> + if (isOrthogonal && (style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())) >> >> I don't understand this comparison "(style().isFlippedBlocksWritingMode() == child.style().isLeftToRightDirection())". > > A bit convoluted maybe. The general idea is that for orthogonal modes SelfStart corresponds to FlexStart with a couple of exceptions, which is when the container has flipped blocks writing mode (vrl or hbt) and the child is LTR, or when it isn't flipped (vlr or htb) but the child is RTL. A couple of examples could illustrate it better: > 1) a HTB ltr child inside a VRL container. In this case the physical left edge is flex item's self-start but flexbox line's flex-end > 2) a HTB RTL child inside a VLR container. In this case the physical left edge is flex item's self-end but flexbox line's flex-start Thanks for the explanation. Maybe add a comment with this. >>>> LayoutTests/TestExpectations:4168 >>>> webkit.org/b/221468 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-align-self-horiz-002.xhtml [ ImageOnlyFailure ] >>> >>> This test is pretty similar to the next one, why is this not passing? >> >> Interesting and good question, checking the test case, there is just one block that is not positioned correctly. I'll try to figure out why. > > It turns out that the test result is correct, it's the expectation the one that is not properly rendered. I'll research why but IMHO it shouldn't block this patch as it is a different bug (the expectations are not using the alignment properties at all). Yeah I don't think that should block anything. But it might be good to report that separated bug and link it from TestExpectations. 1792328 6 svillar 2021-09-10 07:27:06 -0700 Committed r282267 (241545@main): <https://commits.webkit.org/241545@main> 1792368 7 webkit-bug-importer 2021-09-10 09:14:21 -0700 <rdar://problem/82975122> 437489 2021-09-07 05:01:59 -0700 2021-09-10 07:02:08 -0700 Patch bug-229996-20210907140157.patch text/plain 5704 svillar U3VidmVyc2lvbiBSZXZpc2lvbjogMjgyMDc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzNhMjYyMTNmOTIzZjNi YmQ4ODEwZWZiMDU3ZTJjYWRiZGJmZDU3OC4uOWU3NjU4ZDNmNjJhYjQ1MzQwNWRkZTMyZGU0MjU3 OTdlM2IxMTkwZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDIxLTA5LTA3ICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCisgICAgICAgIFtjc3MtZmxl eGJveF0gQWRkIHN1cHBvcnQgZm9yIHNlbGYtc3RhcnQgJiBzZWxmLWVuZCBjc3MtYWxpZ24tMyBw b3NpdGlvbmFsIGFsaWdubWVudCBwcm9wZXJ0aWVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjk5OTYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBBZGRlZCBzdXBwb3J0IGZvciBMZWZ0IGFuZCBSaWdodCBw b3NpdGlvbmFsIGFsaWdubWVudCBwcm9wZXJ0aWVzIGZyb20KKyAgICAgICAgaHR0cHM6Ly9kcmFm dHMuY3Nzd2cub3JnL2Nzcy1hbGlnbi0zLyNwb3NpdGlvbmFsLXZhbHVlcy4gVGhlc2UgdHdvIHBy b3BlcnRpZXMKKyAgICAgICAgYWxpZ24gdGhlIGFsaWdubWVudCBzdWJqZWN0IChmbGV4IGl0ZW0p IHRvIGJlIGZsdXNoIHdpdGggdGhlIGVkZ2Ugb2YgdGhlIGFsaWdubWVudCBjb250YWluZXIgKGZs ZXggbGluZSkKKyAgICAgICAgY29ycmVzcG9uZGluZyB0byB0aGUgYWxpZ25tZW50IHN1YmplY3Qn cyBzdGFydCBzaWRlIGluIHRoZSBhcHByb3ByaWF0ZSBheGlzLiBUaGlzIGFsbG93cyBhdXRob3Jz IHRvIGFsaWduCisgICAgICAgIHRoZSBmbGV4IGl0ZW1zIGJhc2VkIG9uIHRoZWlyIHdyaXRpbmcg bW9kZXMgaW5zdGVhZCBvZiB0aGUgd3JpdGluZyBtb2RlIG9mIHRoZSBjb250YWluZXIuCisKKyAg ICAgICAgKiByZW5kZXJpbmcvUmVuZGVyRmxleGlibGVCb3guY3BwOgorICAgICAgICAoV2ViQ29y ZTo6YWxpZ25tZW50T2Zmc2V0KToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckZsZXhpYmxlQm94 OjphbGlnbm1lbnRGb3JDaGlsZCBjb25zdCk6CisKIDIwMjEtMDktMDEgIFNlcmdpbyBWaWxsYXIg U2VuaW4gIDxzdmlsbGFyQGlnYWxpYS5jb20+CiAKICAgICAgICAgW2Nzcy1mbGV4Ym94XSBBZGQg c3VwcG9ydCBmb3IgbGVmdCAmIHJpZ2h0IGNzcy1hbGlnbi0zIHBvc2l0aW9uYWwgYWxpZ25tZW50 IHByb3BlcnRpZXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJG bGV4aWJsZUJveC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyRmxleGlibGVC b3guY3BwCmluZGV4IGVkYTA3MDBmNzc0NWU4MmY0NDMyZWE3YmZmNzc1ZGM0YzE5ZTIyYTguLjg2 N2VjYWI5ZTFlNGZlNzE3ZDllYmNhNjZlYTNhMmY0NGMzZGY0NjggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJGbGV4aWJsZUJveC5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvcmVuZGVyaW5nL1JlbmRlckZsZXhpYmxlQm94LmNwcApAQCAtMzksNiArMzksNyBAQAog I2luY2x1ZGUgIlJlbmRlckxheW91dFN0YXRlLmgiCiAjaW5jbHVkZSAiUmVuZGVyU3R5bGVDb25z dGFudHMuaCIKICNpbmNsdWRlICJSZW5kZXJWaWV3LmgiCisjaW5jbHVkZSAiV3JpdGluZ01vZGUu aCIKICNpbmNsdWRlIDxsaW1pdHM+CiAjaW5jbHVkZSA8d3RmL0lzb01hbGxvY0lubGluZXMuaD4K ICNpbmNsdWRlIDx3dGYvTWF0aEV4dHJhcy5oPgpAQCAtMTU3MCw2ICsxNTcxLDggQEAgc3RhdGlj IExheW91dFVuaXQgYWxpZ25tZW50T2Zmc2V0KExheW91dFVuaXQgYXZhaWxhYmxlRnJlZVNwYWNl LCBJdGVtUG9zaXRpb24gcG8KICAgICAgICAgYnJlYWs7CiAgICAgY2FzZSBJdGVtUG9zaXRpb246 OlN0YXJ0OgogICAgIGNhc2UgSXRlbVBvc2l0aW9uOjpFbmQ6CisgICAgY2FzZSBJdGVtUG9zaXRp b246OlNlbGZTdGFydDoKKyAgICBjYXNlIEl0ZW1Qb3NpdGlvbjo6U2VsZkVuZDoKICAgICBjYXNl IEl0ZW1Qb3NpdGlvbjo6TGVmdDoKICAgICBjYXNlIEl0ZW1Qb3NpdGlvbjo6UmlnaHQ6CiAgICAg ICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgiJXUgYWxpZ25tZW50Rm9yQ2hpbGQgc2hvdWxkIGhhdmUg dHJhbnNmb3JtZWQgdGhpcyBwb3NpdGlvbiB2YWx1ZSB0byBzb21ldGhpbmcgd2UgaGFuZGxlIGJl bG93LiIsIHN0YXRpY19jYXN0PHVpbnQ4X3Q+KHBvc2l0aW9uKSk7CkBAIC0xNTk2LDggKzE1OTks NiBAQCBzdGF0aWMgTGF5b3V0VW5pdCBhbGlnbm1lbnRPZmZzZXQoTGF5b3V0VW5pdCBhdmFpbGFi bGVGcmVlU3BhY2UsIEl0ZW1Qb3NpdGlvbiBwbwogICAgICAgICAvLyBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTgwNzYKICAgICAgICAgcmV0dXJuIG1heEFzY2VudCAt IGFzY2VudDsKICAgICBjYXNlIEl0ZW1Qb3NpdGlvbjo6TGFzdEJhc2VsaW5lOgotICAgIGNhc2Ug SXRlbVBvc2l0aW9uOjpTZWxmU3RhcnQ6Ci0gICAgY2FzZSBJdGVtUG9zaXRpb246OlNlbGZFbmQ6 CiAgICAgICAgIC8vIEZJWE1FOiBJbXBsZW1lbnQgbGFzdCBiYXNlbGluZS4KICAgICAgICAgYnJl YWs7CiAgICAgfQpAQCAtMTcyMiw2ICsxNzIzLDIxIEBAIEl0ZW1Qb3NpdGlvbiBSZW5kZXJGbGV4 aWJsZUJveDo6YWxpZ25tZW50Rm9yQ2hpbGQoY29uc3QgUmVuZGVyQm94JiBjaGlsZCkgY29uc3QK ICAgICBpZiAoYWxpZ24gPT0gSXRlbVBvc2l0aW9uOjpFbmQpCiAgICAgICAgIHJldHVybiBJdGVt UG9zaXRpb246OkZsZXhFbmQ7CiAKKyAgICBpZiAoYWxpZ24gPT0gSXRlbVBvc2l0aW9uOjpTZWxm U3RhcnQgfHwgYWxpZ24gPT0gSXRlbVBvc2l0aW9uOjpTZWxmRW5kKSB7CisgICAgICAgIGJvb2wg aXNPcnRob2dvbmFsID0gc3R5bGUoKS5pc0hvcml6b250YWxXcml0aW5nTW9kZSgpICE9IGNoaWxk LnN0eWxlKCkuaXNIb3Jpem9udGFsV3JpdGluZ01vZGUoKTsKKyAgICAgICAgaWYgKGlzT3J0aG9n b25hbCAmJiAoc3R5bGUoKS5pc0ZsaXBwZWRCbG9ja3NXcml0aW5nTW9kZSgpID09IGNoaWxkLnN0 eWxlKCkuaXNMZWZ0VG9SaWdodERpcmVjdGlvbigpKSkKKyAgICAgICAgICAgIHJldHVybiBhbGln biA9PSBJdGVtUG9zaXRpb246OlNlbGZTdGFydCA/IEl0ZW1Qb3NpdGlvbjo6RmxleEVuZCA6IEl0 ZW1Qb3NpdGlvbjo6RmxleFN0YXJ0OworCisgICAgICAgIGlmICghaXNPcnRob2dvbmFsKSB7Cisg ICAgICAgICAgICBpZiAoc3R5bGUoKS5pc0ZsaXBwZWRMaW5lc1dyaXRpbmdNb2RlKCkgIT0gY2hp bGQuc3R5bGUoKS5pc0ZsaXBwZWRMaW5lc1dyaXRpbmdNb2RlKCkpCisgICAgICAgICAgICAgICAg cmV0dXJuIGFsaWduID09IEl0ZW1Qb3NpdGlvbjo6U2VsZlN0YXJ0ID8gSXRlbVBvc2l0aW9uOjpG bGV4RW5kIDogSXRlbVBvc2l0aW9uOjpGbGV4U3RhcnQ7CisgICAgICAgICAgICBpZiAoc3R5bGUo KS5pc0xlZnRUb1JpZ2h0RGlyZWN0aW9uKCkgIT0gY2hpbGQuc3R5bGUoKS5pc0xlZnRUb1JpZ2h0 RGlyZWN0aW9uKCkpCisgICAgICAgICAgICAgICAgcmV0dXJuIGFsaWduID09IEl0ZW1Qb3NpdGlv bjo6U2VsZlN0YXJ0ID8gSXRlbVBvc2l0aW9uOjpGbGV4RW5kIDogSXRlbVBvc2l0aW9uOjpGbGV4 U3RhcnQ7CisgICAgICAgIH0KKworICAgICAgICByZXR1cm4gYWxpZ24gPT0gSXRlbVBvc2l0aW9u OjpTZWxmU3RhcnQgPyBJdGVtUG9zaXRpb246OkZsZXhTdGFydCA6IEl0ZW1Qb3NpdGlvbjo6Rmxl eEVuZDsKKyAgICB9CisKICAgICBpZiAoc3R5bGUoKS5mbGV4V3JhcCgpID09IEZsZXhXcmFwOjpS ZXZlcnNlKSB7CiAgICAgICAgIGlmIChhbGlnbiA9PSBJdGVtUG9zaXRpb246OkZsZXhTdGFydCkK ICAgICAgICAgICAgIGFsaWduID0gSXRlbVBvc2l0aW9uOjpGbGV4RW5kOwpkaWZmIC0tZ2l0IGEv TGF5b3V0VGVzdHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IDcwM2U4 ZTBmM2U1ZjUyMDcwNTYxODcyNDkwZmQ2Y2M0MzgzZGQyYjQuLjQ5YzAwMzEzMTM0YjkzZWUzNmI1 MTMxNDQ4YmM1MTFmMDc5ZDYwYzEgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZwor KysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMiBAQAorMjAyMS0wOS0wNyAg U2VyZ2lvIFZpbGxhciBTZW5pbiAgPHN2aWxsYXJAaWdhbGlhLmNvbT4KKworICAgICAgICBbY3Nz LWZsZXhib3hdIEFkZCBzdXBwb3J0IGZvciBzZWxmLXN0YXJ0ICYgc2VsZi1lbmQgY3NzLWFsaWdu LTMgcG9zaXRpb25hbCBhbGlnbm1lbnQgcHJvcGVydGllcworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjI5OTk2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBUZXN0RXhwZWN0YXRpb25zOiBSZW1vdmVkIGEg Y291cGxlIG9mIHRlc3RzIHRoYXQgYXJlIG5vdyBwYXNzaW5nLgorCiAyMDIxLTA5LTAxICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtjc3MtZmxl eGJveF0gQWRkIHN1cHBvcnQgZm9yIGxlZnQgJiByaWdodCBjc3MtYWxpZ24tMyBwb3NpdGlvbmFs IGFsaWdubWVudCBwcm9wZXJ0aWVzCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9UZXN0RXhwZWN0 YXRpb25zIGIvTGF5b3V0VGVzdHMvVGVzdEV4cGVjdGF0aW9ucwppbmRleCBkMDMzYTY5ZDIyNmJj NGE1NzFiOTcwMzBjNjcwZjBlMGRjNjE3OTAwLi42M2JlMGEyMGRkNDMyYzQ5MDM4OTk0YTk1NmIx YTdkYzcwMDU5NWZkIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9UZXN0RXhwZWN0YXRpb25zCisr KyBiL0xheW91dFRlc3RzL1Rlc3RFeHBlY3RhdGlvbnMKQEAgLTQxNjYsOCArNDE2Niw2IEBAIHdl YmtpdC5vcmcvYi8yMjE0NzUgaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3Nz LWZsZXhib3gvZmxleGJveC13cml0CiB3ZWJraXQub3JnL2IvMjIxNDc1IGltcG9ydGVkL3czYy93 ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1mbGV4Ym94L2ZsZXhib3gtd3JpdGluZy1tb2RlLTAx NS5odG1sIFsgSW1hZ2VPbmx5RmFpbHVyZSBdCiAKIHdlYmtpdC5vcmcvYi8yMjE0NjggaW1wb3J0 ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3NzLWZsZXhib3gvZmxleGJveC1hbGlnbi1z ZWxmLWhvcml6LTAwMi54aHRtbCBbIEltYWdlT25seUZhaWx1cmUgXQotd2Via2l0Lm9yZy9iLzIy MTQ2OCBpbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3MtZmxleGJveC9mbGV4 Ym94LWFsaWduLXNlbGYtdmVydC0wMDIueGh0bWwgWyBJbWFnZU9ubHlGYWlsdXJlIF0KLXdlYmtp dC5vcmcvYi8yMjE0NjggaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3NzLWZs ZXhib3gvZmxleGJveC1hbGlnbi1zZWxmLXZlcnQtcnRsLTAwNS54aHRtbCBbIEltYWdlT25seUZh aWx1cmUgXQogCiAjIGFsaWduIGJhc2VsaW5lIGluIGZsZXhib3guCiB3ZWJraXQub3JnL2IvMjIx NDc4IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1mbGV4Ym94L2ZsZXhi b3gtYWxpZ24tc2VsZi1iYXNlbGluZS1ob3Jpei0wMDFhLnhodG1sIFsgSW1hZ2VPbmx5RmFpbHVy ZSBdCg==