79856 2012-02-28 18:54:07 -0800 <msubsup> setNeedsLayout() correction 2012-02-29 17:17:54 -0800 1 1 1 Unclassified WebKit MathML 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 dbarton jchaffraix jchaffraix webkit.review.bot oldest_to_newest 567322 0 dbarton 2012-02-28 18:54:07 -0800 <msubsup> setNeedsLayout() correction 567324 1 129376 dbarton 2012-02-28 19:01:13 -0800 Created attachment 129376 Patch 567327 2 129376 jchaffraix 2012-02-28 19:11:29 -0800 Comment on attachment 129376 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129376&action=review > Source/WebCore/ChangeLog:10 > + superscriptWrapper->setNeedsLayout(true, false); in the last patch actually needs to be > + superscriptWrapper->setNeedsLayout(true); in order to mark its parent, m_scripts, as > + needing layout also. I would rather see a patch with your parent marked in this case. As I told you calling setNeedsLayout(true, true) during layout is really something you don't want to do. That is unless you want to open a security can of worms. > Source/WebCore/ChangeLog:12 > + No new tests. It's actually difficult to test the need for this with the current code, I would really love to see a test for 2 reasons: * this would help your future refactorings. * this will give some confidence that the code is properly tested. 567694 3 129467 dbarton 2012-02-29 10:05:49 -0800 Created attachment 129467 Patch 567699 4 dbarton 2012-02-29 10:10:12 -0800 I changed the code to mark the m_scripts parent. As discussed in private e-mail, the existing tests should cover this fix, except that the current code has a lot of extra setNeedsLayout() calls, so the tests pass anyway. It's hard to come up with a test that wouldn't pass now, but there may be one. We could wait to land this change until we're sure it's needed (the existing tests will start to fail), but that will complicate a future patch, and a potential bug will remain in the meantime. 567767 5 webkit.review.bot 2012-02-29 11:21:26 -0800 Attachment 129467 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 567809 6 129467 jchaffraix 2012-02-29 12:04:32 -0800 Comment on attachment 129467 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129467&action=review >> Source/WebCore/ChangeLog:8 >> + m_scripts->setNeedsLayout(true, false); needs to be added to the fix for bug 79274. > > Line contains tab character. [whitespace/tab] [5] You will not be able to land this. I will fix and submit the patch for landing. 567811 7 129488 jchaffraix 2012-02-29 12:06:40 -0800 Created attachment 129488 Patch for landing 568125 8 129488 webkit.review.bot 2012-02-29 17:17:50 -0800 Comment on attachment 129488 Patch for landing Clearing flags on attachment: 129488 Committed r109286: <http://trac.webkit.org/changeset/109286> 568126 9 webkit.review.bot 2012-02-29 17:17:54 -0800 All reviewed patches have been landed. Closing bug. 129376 2012-02-28 19:01:13 -0800 2012-02-29 10:05:17 -0800 Patch bug-79856-20120228190111.patch text/plain 2025 dbarton SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwOTE4MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI0IEBACisyMDEyLTAyLTI4ICBEYXZpZCBC YXJ0b24gIDxkYmFydG9uQG1hdGhzY3JpYmUuY29tPgorCisgICAgICAgIDxtc3Vic3VwPiBzZXRO ZWVkc0xheW91dCgpIGNvcnJlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTc5ODU2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgc3VwZXJzY3JpcHRXcmFwcGVyLT5zZXROZWVkc0xheW91dCh0cnVlLCBm YWxzZSk7IGluIHRoZSBsYXN0IHBhdGNoIGFjdHVhbGx5IG5lZWRzIHRvIGJlCisgICAgICAgIHN1 cGVyc2NyaXB0V3JhcHBlci0+c2V0TmVlZHNMYXlvdXQodHJ1ZSk7IGluIG9yZGVyIHRvIG1hcmsg aXRzIHBhcmVudCwgbV9zY3JpcHRzLCBhcworICAgICAgICBuZWVkaW5nIGxheW91dCBhbHNvLgor CisgICAgICAgIE5vIG5ldyB0ZXN0cy4gSXQncyBhY3R1YWxseSBkaWZmaWN1bHQgdG8gdGVzdCB0 aGUgbmVlZCBmb3IgdGhpcyB3aXRoIHRoZSBjdXJyZW50IGNvZGUsCisgICAgICAgIGJlY2F1c2Ug dGhlcmUgYXJlIGEgbG90IG9mIGV4dHJhIHNldE5lZWRzTGF5b3V0KCkgY2FsbHMgcmlnaHQgbm93 LiBJIGNhdWdodCB0aGlzIGFmdGVyCisgICAgICAgIHJlbW92aW5nIHNvbWUgb2YgdGhvc2UgY2Fs bHMgdGhhdCBzaG91bGQgYmUgcmVkdW5kYW50LCBmb3IgYSBmdXR1cmUgcGF0Y2guIEFjdHVhbGx5 CisgICAgICAgIHJlbW92aW5nIHRob3NlIGNhbGxzIG5vdyBjaGFuZ2VzIG90aGVyIGJlaGF2aW9y LCBkdWUgdG8gb3RoZXIgYnVncywgc28gSSdsbCBzYXZlIGFsbAorICAgICAgICB0aGF0IGZvciBm dXR1cmUgcGF0Y2hlcy4gV2Ugc2hvdWxkIGxhbmQgdGhpcyBmaXggbm93IHRob3VnaCwgYXMgaXQg Y291bGQgcG9zc2libHkgbWF0dGVyCisgICAgICAgIGluIHNvbWUgY2FzZXMuCisKKyAgICAgICAg KiByZW5kZXJpbmcvbWF0aG1sL1JlbmRlck1hdGhNTFN1YlN1cC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpSZW5kZXJNYXRoTUxTdWJTdXA6OmxheW91dCk6CisKIDIwMTItMDItMjggIEFuZGVycyBD YXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBXaXRoIHRpbGVkIGRyYXdp bmcgZW5hYmxlZCwgcHJlc3NpbmcgRG93biBhcnJvdyBhZnRlciBzY3JvbGxpbmcgdmlhIG1vdXNl IGdlc3R1cmUgY2F1c2VzIHBhZ2UgdG8ganVtcCBiYWNrIHVwIHRvIHRvcApJbmRleDogU291cmNl L1dlYkNvcmUvcmVuZGVyaW5nL21hdGhtbC9SZW5kZXJNYXRoTUxTdWJTdXAuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9tYXRobWwvUmVuZGVyTWF0aE1MU3ViU3Vw LmNwcAkocmV2aXNpb24gMTA5MTA3KQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL21hdGht bC9SZW5kZXJNYXRoTUxTdWJTdXAuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xODIsNyArMTgyLDcg QEAgdm9pZCBSZW5kZXJNYXRoTUxTdWJTdXA6OmxheW91dCgpCiAgICAgYmFzZVdyYXBwZXItPnNl dE5lZWRzTGF5b3V0KHRydWUsIGZhbHNlKTsKICAgICAKICAgICBzdXBlcnNjcmlwdFdyYXBwZXIt PnN0eWxlKCktPnNldFBhZGRpbmdCb3R0b20oTGVuZ3RoKHN1cGVyUGFkZGluZ0JvdHRvbSwgRml4 ZWQpKTsKLSAgICBzdXBlcnNjcmlwdFdyYXBwZXItPnNldE5lZWRzTGF5b3V0KHRydWUsIGZhbHNl KTsKKyAgICBzdXBlcnNjcmlwdFdyYXBwZXItPnNldE5lZWRzTGF5b3V0KHRydWUpOwogICAgIAog ICAgIFJlbmRlckJsb2NrOjpsYXlvdXQoKTsKIH0K 129467 2012-02-29 10:05:49 -0800 2012-02-29 12:06:30 -0800 Patch bug-79856-20120229100548.patch text/plain 1807 dbarton SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwOTIyNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDEyLTAyLTI5ICBEYXZpZCBC YXJ0b24gIDxkYmFydG9uQG1hdGhzY3JpYmUuY29tPgorCisgICAgICAgIDxtc3Vic3VwPiBzZXRO ZWVkc0xheW91dCgpIGNvcnJlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTc5ODU2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAJbV9zY3JpcHRzLT5zZXROZWVkc0xheW91dCh0cnVlLCBmYWxzZSk7IG5lZWRz IHRvIGJlIGFkZGVkIHRvIHRoZSBmaXggZm9yIGJ1ZyA3OTI3NC4KKworICAgICAgICBObyBuZXcg dGVzdHMuIEl0J3MgYWN0dWFsbHkgZGlmZmljdWx0IHRvIHRlc3QgdGhlIG5lZWQgZm9yIHRoaXMg d2l0aCB0aGUgY3VycmVudCBjb2RlLAorICAgICAgICBiZWNhdXNlIHRoZXJlIGFyZSBhIGxvdCBv ZiBleHRyYSBzZXROZWVkc0xheW91dCgpIGNhbGxzIHJpZ2h0IG5vdy4gSSBjYXVnaHQgdGhpcyBh ZnRlcgorICAgICAgICByZW1vdmluZyBzb21lIG9mIHRob3NlIGNhbGxzIHRoYXQgc2hvdWxkIGJl IHJlZHVuZGFudCwgZm9yIGEgZnV0dXJlIHBhdGNoLiBBY3R1YWxseQorICAgICAgICByZW1vdmlu ZyB0aG9zZSBjYWxscyBub3cgY2hhbmdlcyBvdGhlciBiZWhhdmlvciwgZHVlIHRvIG90aGVyIGJ1 Z3MsIHNvIEknbGwgc2F2ZSBhbGwKKyAgICAgICAgdGhhdCBmb3IgZnV0dXJlIHBhdGNoZXMuIFdl IHNob3VsZCBsYW5kIHRoaXMgZml4IG5vdyB0aG91Z2ggSSB0aGluaywgYXMgaXQgY291bGQKKyAg ICAgICAgcG9zc2libHkgbWF0dGVyIGluIHNvbWUgY2FzZXMuCisKKyAgICAgICAgKiByZW5kZXJp bmcvbWF0aG1sL1JlbmRlck1hdGhNTFN1YlN1cC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5k ZXJNYXRoTUxTdWJTdXA6OmxheW91dCk6CisKIDIwMTItMDItMjcgIFZzZXZvbG9kIFZsYXNvdiAg PHZzZXZpa0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogW0luc3BlY3Rv ckluZGV4ZWREQl0gQWRkIHJlZnJlc2ggdG8gSW5kZXhlZERCIHN1cHBvcnQuCkluZGV4OiBTb3Vy Y2UvV2ViQ29yZS9yZW5kZXJpbmcvbWF0aG1sL1JlbmRlck1hdGhNTFN1YlN1cC5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL21hdGhtbC9SZW5kZXJNYXRoTUxTdWJT dXAuY3BwCShyZXZpc2lvbiAxMDkxMDcpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvbWF0 aG1sL1JlbmRlck1hdGhNTFN1YlN1cC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE4Myw2ICsxODMs NyBAQCB2b2lkIFJlbmRlck1hdGhNTFN1YlN1cDo6bGF5b3V0KCkKICAgICAKICAgICBzdXBlcnNj cmlwdFdyYXBwZXItPnN0eWxlKCktPnNldFBhZGRpbmdCb3R0b20oTGVuZ3RoKHN1cGVyUGFkZGlu Z0JvdHRvbSwgRml4ZWQpKTsKICAgICBzdXBlcnNjcmlwdFdyYXBwZXItPnNldE5lZWRzTGF5b3V0 KHRydWUsIGZhbHNlKTsKKyAgICBtX3NjcmlwdHMtPnNldE5lZWRzTGF5b3V0KHRydWUsIGZhbHNl KTsKICAgICAKICAgICBSZW5kZXJCbG9jazo6bGF5b3V0KCk7CiB9Cg== 129488 2012-02-29 12:06:40 -0800 2012-02-29 17:17:50 -0800 Patch for landing bug-79856-20120229120639.patch text/plain 1912 jchaffraix U3VidmVyc2lvbiBSZXZpc2lvbjogMTA5MjQyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjhiZWI1MWMyYmRiMTJm NjQzMmYxMjAzMjQ2ZTdiYTc3NzA5ZDdkOS4uNzg3MTA3NThiMTBlN2IxODUxZTJjOTZjNzI3Zjc1 NmU1YjE2YmE5OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEyLTAyLTI5ICBEYXZp ZCBCYXJ0b24gIDxkYmFydG9uQG1hdGhzY3JpYmUuY29tPgorCisgICAgICAgIDxtc3Vic3VwPiBz ZXROZWVkc0xheW91dCgpIGNvcnJlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTc5ODU2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgSnVsaWVuIENo YWZmcmFpeC4KKworICAgICAgICBtX3NjcmlwdHMtPnNldE5lZWRzTGF5b3V0KHRydWUsIGZhbHNl KTsgbmVlZHMgdG8gYmUgYWRkZWQgdG8gdGhlIGZpeCBmb3IgYnVnIDc5Mjc0LgorCisgICAgICAg IE5vIG5ldyB0ZXN0cy4gSXQncyBhY3R1YWxseSBkaWZmaWN1bHQgdG8gdGVzdCB0aGUgbmVlZCBm b3IgdGhpcyB3aXRoIHRoZSBjdXJyZW50IGNvZGUsCisgICAgICAgIGJlY2F1c2UgdGhlcmUgYXJl IGEgbG90IG9mIGV4dHJhIHNldE5lZWRzTGF5b3V0KCkgY2FsbHMgcmlnaHQgbm93LiBJIGNhdWdo dCB0aGlzIGFmdGVyCisgICAgICAgIHJlbW92aW5nIHNvbWUgb2YgdGhvc2UgY2FsbHMgdGhhdCBz aG91bGQgYmUgcmVkdW5kYW50LCBmb3IgYSBmdXR1cmUgcGF0Y2guIEFjdHVhbGx5CisgICAgICAg IHJlbW92aW5nIHRob3NlIGNhbGxzIG5vdyBjaGFuZ2VzIG90aGVyIGJlaGF2aW9yLCBkdWUgdG8g b3RoZXIgYnVncywgc28gSSdsbCBzYXZlIGFsbAorICAgICAgICB0aGF0IGZvciBmdXR1cmUgcGF0 Y2hlcy4gV2Ugc2hvdWxkIGxhbmQgdGhpcyBmaXggbm93IHRob3VnaCBJIHRoaW5rLCBhcyBpdCBj b3VsZAorICAgICAgICBwb3NzaWJseSBtYXR0ZXIgaW4gc29tZSBjYXNlcy4KKworICAgICAgICAq IHJlbmRlcmluZy9tYXRobWwvUmVuZGVyTWF0aE1MU3ViU3VwLmNwcDoKKyAgICAgICAgKFdlYkNv cmU6OlJlbmRlck1hdGhNTFN1YlN1cDo6bGF5b3V0KToKKwogMjAxMi0wMi0yOSAgTWF4IEZlaWwg IDxtZmVpbEByaW0uY29tPgogCiAgICAgICAgIFtCbGFja0JlcnJ5XSBBZGQgc3VwcG9ydCBmb3Ig RkxBQyBhdWRpbyBhbmQgT0dHL1ZvcmJpcyBhdWRpbwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvcmVuZGVyaW5nL21hdGhtbC9SZW5kZXJNYXRoTUxTdWJTdXAuY3BwIGIvU291cmNlL1dlYkNv cmUvcmVuZGVyaW5nL21hdGhtbC9SZW5kZXJNYXRoTUxTdWJTdXAuY3BwCmluZGV4IGE4YjVhZjY1 ZDFkMTkzMTExMDY0YzQ2MzQyMzUyY2ZjZDk3NDJkMWMuLjg0OTI4ZDRiNDhlYjNjMjdjYWVlZDQ1 Zjc4YzliNDMyZDgyMjcyNDIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9t YXRobWwvUmVuZGVyTWF0aE1MU3ViU3VwLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJp bmcvbWF0aG1sL1JlbmRlck1hdGhNTFN1YlN1cC5jcHAKQEAgLTE4Myw2ICsxODMsNyBAQCB2b2lk IFJlbmRlck1hdGhNTFN1YlN1cDo6bGF5b3V0KCkKICAgICAKICAgICBzdXBlcnNjcmlwdFdyYXBw ZXItPnN0eWxlKCktPnNldFBhZGRpbmdCb3R0b20oTGVuZ3RoKHN1cGVyUGFkZGluZ0JvdHRvbSwg Rml4ZWQpKTsKICAgICBzdXBlcnNjcmlwdFdyYXBwZXItPnNldE5lZWRzTGF5b3V0KHRydWUsIGZh bHNlKTsKKyAgICBtX3NjcmlwdHMtPnNldE5lZWRzTGF5b3V0KHRydWUsIGZhbHNlKTsKICAgICAK ICAgICBSZW5kZXJCbG9jazo6bGF5b3V0KCk7CiB9Cg==