88049 2012-06-01 00:01:35 -0700 Layout not updated after setting -webkit-line-clamp to none 2012-06-01 07:14:32 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mitz webkit-unassigned eric inferno webkit.review.bot oldest_to_newest 639016 0 mitz 2012-06-01 00:01:35 -0700 <rdar://problem/11407365> Changing -webkit-line-clamp dynamically from a number or percentage value to none doesn’t remove clamping. Patch forthcoming. 639027 1 145222 mitz 2012-06-01 00:16:26 -0700 Created attachment 145222 Mark blocks for layout and clear truncation when line-clamp changes to none 639064 2 145222 inferno 2012-06-01 00:58:06 -0700 Comment on attachment 145222 Mark blocks for layout and clear truncation when line-clamp changes to none View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:153 > +void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle *newStyle) * on the wrong side. > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:987 > +void RenderDeprecatedFlexibleBox::clearLineClamp() nit: Can this code be shared with http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp&exact_package=chromium&q=markPositionedObjectsForLayout&type=cs&l=883 to avoid duplication. > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:995 > + || (child->style()->height().isAuto() && child->isBlockFlow())) { Do we want to use logicalHeight here ? > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:996 > + child->setChildNeedsLayout(true); I think we just need to MarkOnlyThis in the second argument. 639067 3 145222 inferno 2012-06-01 01:03:43 -0700 Comment on attachment 145222 Mark blocks for layout and clear truncation when line-clamp changes to none View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review > Source/WebCore/ChangeLog:14 > + children for layout and clears trunaction from blocks. typo 'trunacation' 639081 4 145222 mitz 2012-06-01 01:18:28 -0700 Comment on attachment 145222 Mark blocks for layout and clear truncation when line-clamp changes to none View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review Thanks for the review! >> Source/WebCore/ChangeLog:14 >> + children for layout and clears trunaction from blocks. > > typo 'trunacation' I am going to fix this. >> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:153 >> +void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle *newStyle) > > * on the wrong side. Oops! I will fix this. >> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:995 >> + || (child->style()->height().isAuto() && child->isBlockFlow())) { > > Do we want to use logicalHeight here ? I don’t think RenderDeprecatedFlexibleBox was ever made writing-mode aware. More importantly, this code follows the logic in applyLineClamp(). >> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:996 >> + child->setChildNeedsLayout(true); > > I think we just need to MarkOnlyThis in the second argument. MarkOnlyThis is only useful during layout, when you don’t want to re-dirty the ancestors. clearLineClamp() is not called during layout, so MarkOnlyThis is not necessary and using it would introduce risk of creating a subtree that needs layout and never gets it. 639085 5 inferno 2012-06-01 01:21:46 -0700 Sounds good. Thanks for the patch. 639308 6 mitz 2012-06-01 07:14:32 -0700 Fixed in <http://trac.webkit.org/r119227>. 145222 2012-06-01 00:16:26 -0700 2012-06-01 01:18:28 -0700 Mark blocks for layout and clear truncation when line-clamp changes to none 11407365_r1.diff text/plain 5579 mitz SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExOTE5NikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA2LTAxICBEYW4gQmVy bnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CisKKyAgICAgICAgTGF5b3V0IG5vdCB1cGRhdGVkIGFm dGVyIHNldHRpbmcgLXdlYmtpdC1saW5lLWNsYW1wIHRvIG5vbmUKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTg4MDQ5CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGVzdDogZmFzdC9mbGV4Ym94L2xpbmUtY2xh bXAtcmVtb3ZlZC1keW5hbWljYWxseS5odG1sCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVy RGVwcmVjYXRlZEZsZXhpYmxlQm94LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckRlcHJl Y2F0ZWRGbGV4aWJsZUJveDo6c3R5bGVXaWxsQ2hhbmdlKTogQWRkZWQuIENhbGxzIGNsZWFyTGlu ZUNsYW1wIGlmCisgICAgICAgIGxpbmUtY2xhbXAgd2lsbCBjaGFuZ2UgdG8gbm9uZS4KKyAgICAg ICAgKFdlYkNvcmU6OlJlbmRlckRlcHJlY2F0ZWRGbGV4aWJsZUJveDo6Y2xlYXJMaW5lQ2xhbXAp OiBBZGRlZC4gTWFya3MgcG9zc2libHktY2xhbXBlZAorICAgICAgICBjaGlsZHJlbiBmb3IgbGF5 b3V0IGFuZCBjbGVhcnMgdHJ1bmFjdGlvbiBmcm9tIGJsb2Nrcy4KKyAgICAgICAgKiByZW5kZXJp bmcvUmVuZGVyRGVwcmVjYXRlZEZsZXhpYmxlQm94Lmg6CisKIDIwMTItMDUtMzEgIERvbWluaWMg Q29vbmV5ICA8ZG9taW5pY2NAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFtWOF0gRXhwYW5kbyBw cm9wZXJ0aWVzIG9uIGF0dHJpYnV0ZSBub2RlcyBkaXNhcHBlYXIKSW5kZXg6IFNvdXJjZS9XZWJD b3JlL3JlbmRlcmluZy9SZW5kZXJEZXByZWNhdGVkRmxleGlibGVCb3guY3BwCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJEZXByZWNhdGVkRmxleGlibGVCb3gu Y3BwCShyZXZpc2lvbiAxMTkxODYpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVy RGVwcmVjYXRlZEZsZXhpYmxlQm94LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTUwLDYgKzE1MCwx NSBAQCBzdGF0aWMgYm9vbCBjaGlsZERvZXNOb3RBZmZlY3RXaWR0aE9yRmxlCiAgICAgcmV0dXJu IGNoaWxkLT5pc1Bvc2l0aW9uZWQoKSB8fCBjaGlsZC0+c3R5bGUoKS0+dmlzaWJpbGl0eSgpID09 IENPTExBUFNFOwogfQogCit2b2lkIFJlbmRlckRlcHJlY2F0ZWRGbGV4aWJsZUJveDo6c3R5bGVX aWxsQ2hhbmdlKFN0eWxlRGlmZmVyZW5jZSBkaWZmLCBjb25zdCBSZW5kZXJTdHlsZSAqbmV3U3R5 bGUpCit7CisgICAgUmVuZGVyU3R5bGUqIG9sZFN0eWxlID0gc3R5bGUoKTsKKyAgICBpZiAob2xk U3R5bGUgJiYgIW9sZFN0eWxlLT5saW5lQ2xhbXAoKS5pc05vbmUoKSAmJiBuZXdTdHlsZS0+bGlu ZUNsYW1wKCkuaXNOb25lKCkpCisgICAgICAgIGNsZWFyTGluZUNsYW1wKCk7CisKKyAgICBSZW5k ZXJCbG9jazo6c3R5bGVXaWxsQ2hhbmdlKGRpZmYsIG5ld1N0eWxlKTsKK30KKwogdm9pZCBSZW5k ZXJEZXByZWNhdGVkRmxleGlibGVCb3g6OmNhbGNIb3Jpem9udGFsUHJlZldpZHRocygpCiB7CiAg ICAgZm9yIChSZW5kZXJCb3gqIGNoaWxkID0gZmlyc3RDaGlsZEJveCgpOyBjaGlsZDsgY2hpbGQg PSBjaGlsZC0+bmV4dFNpYmxpbmdCb3goKSkgewpAQCAtOTc1LDYgKzk4NCwyNSBAQCB2b2lkIFJl bmRlckRlcHJlY2F0ZWRGbGV4aWJsZUJveDo6YXBwbHlMCiAgICAgfQogfQogCit2b2lkIFJlbmRl ckRlcHJlY2F0ZWRGbGV4aWJsZUJveDo6Y2xlYXJMaW5lQ2xhbXAoKQoreworICAgIEZsZXhCb3hJ dGVyYXRvciBpdGVyYXRvcih0aGlzKTsKKyAgICBmb3IgKFJlbmRlckJveCogY2hpbGQgPSBpdGVy YXRvci5maXJzdCgpOyBjaGlsZDsgY2hpbGQgPSBpdGVyYXRvci5uZXh0KCkpIHsKKyAgICAgICAg aWYgKGNoaWxkRG9lc05vdEFmZmVjdFdpZHRoT3JGbGV4aW5nKGNoaWxkKSkKKyAgICAgICAgICAg IGNvbnRpbnVlOworCisgICAgICAgIGlmICgoY2hpbGQtPmlzUmVwbGFjZWQoKSAmJiAoY2hpbGQt PnN0eWxlKCktPndpZHRoKCkuaXNQZXJjZW50KCkgfHwgY2hpbGQtPnN0eWxlKCktPmhlaWdodCgp LmlzUGVyY2VudCgpKSkKKyAgICAgICAgICAgIHx8IChjaGlsZC0+c3R5bGUoKS0+aGVpZ2h0KCku aXNBdXRvKCkgJiYgY2hpbGQtPmlzQmxvY2tGbG93KCkpKSB7CisgICAgICAgICAgICBjaGlsZC0+ c2V0Q2hpbGROZWVkc0xheW91dCh0cnVlKTsKKworICAgICAgICAgICAgaWYgKGNoaWxkLT5pc1Jl bmRlckJsb2NrKCkpIHsKKyAgICAgICAgICAgICAgICB0b1JlbmRlckJsb2NrKGNoaWxkKS0+bWFy a1Bvc2l0aW9uZWRPYmplY3RzRm9yTGF5b3V0KCk7CisgICAgICAgICAgICAgICAgdG9SZW5kZXJC bG9jayhjaGlsZCktPmNsZWFyVHJ1bmNhdGlvbigpOworICAgICAgICAgICAgfQorICAgICAgICB9 CisgICAgfQorfQorCiB2b2lkIFJlbmRlckRlcHJlY2F0ZWRGbGV4aWJsZUJveDo6cGxhY2VDaGls ZChSZW5kZXJCb3gqIGNoaWxkLCBjb25zdCBMYXlvdXRQb2ludCYgbG9jYXRpb24pCiB7CiAgICAg TGF5b3V0UmVjdCBvbGRSZWN0ID0gY2hpbGQtPmZyYW1lUmVjdCgpOwpJbmRleDogU291cmNlL1dl YkNvcmUvcmVuZGVyaW5nL1JlbmRlckRlcHJlY2F0ZWRGbGV4aWJsZUJveC5oCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJEZXByZWNhdGVkRmxleGlibGVCb3gu aAkocmV2aXNpb24gMTE5MTg2KQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckRl cHJlY2F0ZWRGbGV4aWJsZUJveC5oCSh3b3JraW5nIGNvcHkpCkBAIC00MCw2ICs0MCw4IEBAIHB1 YmxpYzoKICAgICB2b2lkIGNhbGNIb3Jpem9udGFsUHJlZldpZHRocygpOwogICAgIHZvaWQgY2Fs Y1ZlcnRpY2FsUHJlZldpZHRocygpOwogCisgICAgdmlydHVhbCB2b2lkIHN0eWxlV2lsbENoYW5n ZShTdHlsZURpZmZlcmVuY2UsIGNvbnN0IFJlbmRlclN0eWxlKiBuZXdTdHlsZSkgT1ZFUlJJREU7 CisKICAgICB2aXJ0dWFsIHZvaWQgbGF5b3V0QmxvY2soYm9vbCByZWxheW91dENoaWxkcmVuLCBM YXlvdXRVbml0IHBhZ2VIZWlnaHQgPSAwKTsKICAgICB2b2lkIGxheW91dEhvcml6b250YWxCb3go Ym9vbCByZWxheW91dENoaWxkcmVuKTsKICAgICB2b2lkIGxheW91dFZlcnRpY2FsQm94KGJvb2wg cmVsYXlvdXRDaGlsZHJlbik7CkBAIC02NCw2ICs2Niw3IEBAIHByb3RlY3RlZDoKIAogcHJpdmF0 ZToKICAgICB2b2lkIGFwcGx5TGluZUNsYW1wKEZsZXhCb3hJdGVyYXRvciYsIGJvb2wgcmVsYXlv dXRDaGlsZHJlbik7CisgICAgdm9pZCBjbGVhckxpbmVDbGFtcCgpOwogfTsKIAogfSAvLyBuYW1l c3BhY2UgV2ViQ29yZQpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IExheW91dFRlc3RzL0NoYW5nZUxvZwkocmV2aXNpb24gMTE5MTk2KQorKysgTGF5b3V0VGVzdHMv Q2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTItMDYtMDEgIERh biBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KKworICAgICAgICBMYXlvdXQgbm90IHVwZGF0 ZWQgYWZ0ZXIgc2V0dGluZyAtd2Via2l0LWxpbmUtY2xhbXAgdG8gbm9uZQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODgwNDkKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGZhc3QvZmxleGJveC9saW5lLWNs YW1wLXJlbW92ZWQtZHluYW1pY2FsbHktZXhwZWN0ZWQuaHRtbDogQWRkZWQuCisgICAgICAgICog ZmFzdC9mbGV4Ym94L2xpbmUtY2xhbXAtcmVtb3ZlZC1keW5hbWljYWxseS5odG1sOiBBZGRlZC4K KwogMjAxMi0wNS0zMSAgRG9taW5pYyBDb29uZXkgIDxkb21pbmljY0BjaHJvbWl1bS5vcmc+CiAK ICAgICAgICAgW1Y4XSBFeHBhbmRvIHByb3BlcnRpZXMgb24gYXR0cmlidXRlIG5vZGVzIGRpc2Fw cGVhcgpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9mbGV4Ym94L2xpbmUtY2xhbXAtcmVtb3ZlZC1k eW5hbWljYWxseS1leHBlY3RlZC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3Qv ZmxleGJveC9saW5lLWNsYW1wLXJlbW92ZWQtZHluYW1pY2FsbHktZXhwZWN0ZWQuaHRtbAkocmV2 aXNpb24gMCkKKysrIExheW91dFRlc3RzL2Zhc3QvZmxleGJveC9saW5lLWNsYW1wLXJlbW92ZWQt ZHluYW1pY2FsbHktZXhwZWN0ZWQuaHRtbAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwxMCBAQAor PGRpdiBpZD0idGFyZ2V0IiBzdHlsZT0iCisgICAgZGlzcGxheTogLXdlYmtpdC1ib3g7CisgICAg LXdlYmtpdC1ib3gtb3JpZW50OiB2ZXJ0aWNhbDsKKyAgICBvdmVyZmxvdzogaGlkZGVuOworICAg IHdpZHRoOiAxMDBweDsKKyI+CisgICAgPGRpdj4KKyAgICAgICAgVW5sZXNzIHRoaXMgc2VudGVu Y2UgaXMgdHJ1bmNhdGVkIGFmdGVyIHR3byBsaW5lcywgUEFTUy4KKyAgICA8L2Rpdj4KKzwvZGl2 PgpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9mbGV4Ym94L2xpbmUtY2xhbXAtcmVtb3ZlZC1keW5h bWljYWxseS5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3QvZmxleGJveC9saW5l LWNsYW1wLXJlbW92ZWQtZHluYW1pY2FsbHkuaHRtbAkocmV2aXNpb24gMCkKKysrIExheW91dFRl c3RzL2Zhc3QvZmxleGJveC9saW5lLWNsYW1wLXJlbW92ZWQtZHluYW1pY2FsbHkuaHRtbAkocmV2 aXNpb24gMCkKQEAgLTAsMCArMSwxNSBAQAorPGRpdiBpZD0idGFyZ2V0IiBzdHlsZT0iCisgICAg ZGlzcGxheTogLXdlYmtpdC1ib3g7CisgICAgLXdlYmtpdC1ib3gtb3JpZW50OiB2ZXJ0aWNhbDsK KyAgICBvdmVyZmxvdzogaGlkZGVuOworICAgIC13ZWJraXQtbGluZS1jbGFtcDogMjsKKyAgICB3 aWR0aDogMTAwcHg7CisiPgorICAgIDxkaXY+CisgICAgICAgIFVubGVzcyB0aGlzIHNlbnRlbmNl IGlzIHRydW5jYXRlZCBhZnRlciB0d28gbGluZXMsIFBBU1MuCisgICAgPC9kaXY+Cis8L2Rpdj4K KzxzY3JpcHQ+CisgICAgZG9jdW1lbnQuYm9keS5vZmZzZXRUb3A7CisgICAgdGFyZ2V0LnN0eWxl LnJlbW92ZVByb3BlcnR5KCItd2Via2l0LWxpbmUtY2xhbXAiKTsKKzwvc2NyaXB0Pgo=