33600 2010-01-13 08:20:03 -0800 RenderObject::nextInPreOrderAfterChildren(RenderObject* stayWithin) does not stay within 2010-01-13 12:40:35 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 11031 1 carol carol commit-queue darin laszlo.gombos oldest_to_newest 180232 0 carol 2010-01-13 08:20:03 -0800 This function does not perform as expected, if the stayWithin renderer has a nextSibling. The function when called repeatedly shall traverse till the end of the tree. This may not have many visible consequences (except for CSS counters which are broken anyway until 32884 is fixed), but it should have a definite impact on performance. 180233 1 carol 2010-01-13 08:21:43 -0800 After 32884 is fixed this w3 test still does not pass as a result of this bug: http://www.w3.org/Style/CSS/Test/CSS2.1/current/html4/counters-scope-implied-001.htm 180249 2 darin 2010-01-13 08:59:03 -0800 We should fix this bug right away! Seems pretty straightforward. Presumably the code should match Node::traverseNextSibling which is doing the same sort of thing. 180251 3 carol 2010-01-13 09:02:00 -0800 I shall submit a patch within the next hour. With no LayoutTest attached if this is OK. 180260 4 darin 2010-01-13 09:43:36 -0800 (In reply to comment #3) > I shall submit a patch within the next hour. With no LayoutTest attached if > this is OK. We can accept the patch with no regression test on the basis that the counters test case will cover it in the near future. Normally we go out of our way to avoid this sort of thing, and find a creative way to demonstrate the bug, but I assume you have not found way to test this before adding the new counters code. 180287 5 carol 2010-01-13 10:46:02 -0800 I made a fix, but apparently it crashes all editing/deleting LayoutTests. I'll have to look into this so the fix will take longer than expected. 180326 6 46482 carol 2010-01-13 12:09:32 -0800 Created attachment 46482 Proposed patch To the best of my knowledge the code I changed here conforms to the WebKit style specification, but the file itself has many style problems which I did not undertake to fix. 180327 7 carol 2010-01-13 12:13:56 -0800 (In reply to comment #5) > I made a fix, but apparently it crashes all editing/deleting LayoutTests. I'll > have to look into this so the fix will take longer than expected. The problems I noticed with the editing/deleting LayoutTests were due to a typo in my code which I fixed and verified before I have submitted the patch. Sorry for taking so long for such a trivial issue. 180349 8 46482 commit-queue 2010-01-13 12:40:29 -0800 Comment on attachment 46482 Proposed patch Clearing flags on attachment: 46482 Committed r53196: <http://trac.webkit.org/changeset/53196> 180350 9 commit-queue 2010-01-13 12:40:35 -0800 All reviewed patches have been landed. Closing bug. 46482 2010-01-13 12:09:32 -0800 2010-01-13 12:40:29 -0800 Proposed patch patchFor33600-4.patch text/plain 1886 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MzE4MikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMTAtMDEtMTMgIENhcm9sIFN6YWJvICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFJlbmRlck9iamVjdDo6bmV4dEluUHJlT3JkZXJBZnRlckNoaWxkcmVuKFJlbmRlck9iamVjdCog c3RheVdpdGhpbikgZG9lcyBub3Qgc3RheSB3aXRoaW4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMzNjAwCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIGFz IHRoaXMgZml4IGFmZmVjdHMgbWFpbmx5IHBlcmZvcm1hbmNlLgorICAgICAgICBObyB0ZXN0IGhh cyBiZWVuIGZvdW5kIHlldCB0aGF0IGNvdWxkIGV2aWRlbmNlIHRoZSBidWcgaW4gdGhlIGxheW91 dAorICAgICAgICBvZiBhIHBhZ2UsIGJ1dCB0aGUgZml4IHRvIGJ1ZyAzMjg4NCBleHBvc2VzIHRo aXMgYnVnIG9uIHNvbWUgcGFnZXMKKyAgICAgICAgc3VjaCBhcyBodHRwOi8vd3d3LnczLm9yZy9T dHlsZS9DU1MvVGVzdC9DU1MyLjEvY3VycmVudC9odG1sNC9jb3VudGVycy1zY29wZS1pbXBsaWVk LTAwMS5odG0KKworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJPYmplY3QuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6UmVuZGVyT2JqZWN0OjpuZXh0SW5QcmVPcmRlckFmdGVyQ2hpbGRyZW4pOgor CiAyMDEwLTAxLTEzICBNaWlra2EgSGVpa2tpbmVuIDxtaWlra2EuaGVpa2tpbmVuQGRpZ2lhLmNv bT4KIAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBIYXVzbWFubi4KSW5kZXg6IFdlYkNvcmUv cmVuZGVyaW5nL1JlbmRlck9iamVjdC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9yZW5kZXJp bmcvUmVuZGVyT2JqZWN0LmNwcAkocmV2aXNpb24gNTMxODIpCisrKyBXZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJPYmplY3QuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zNjgsMTkgKzM2OCwxNCBAQCBS ZW5kZXJPYmplY3QqIFJlbmRlck9iamVjdDo6bmV4dEluUHJlT3JkCiAgICAgaWYgKHRoaXMgPT0g c3RheVdpdGhpbikKICAgICAgICAgcmV0dXJuIDA7CiAKLSAgICBSZW5kZXJPYmplY3QqIG87Ci0g ICAgaWYgKCEobyA9IG5leHRTaWJsaW5nKCkpKSB7Ci0gICAgICAgIG8gPSBwYXJlbnQoKTsKLSAg ICAgICAgd2hpbGUgKG8gJiYgIW8tPm5leHRTaWJsaW5nKCkpIHsKLSAgICAgICAgICAgIGlmIChv ID09IHN0YXlXaXRoaW4pCi0gICAgICAgICAgICAgICAgcmV0dXJuIDA7Ci0gICAgICAgICAgICBv ID0gby0+cGFyZW50KCk7Ci0gICAgICAgIH0KLSAgICAgICAgaWYgKG8pCi0gICAgICAgICAgICBv ID0gby0+bmV4dFNpYmxpbmcoKTsKKyAgICBjb25zdCBSZW5kZXJPYmplY3QqIGN1cnJlbnQgPSB0 aGlzOworICAgIFJlbmRlck9iamVjdCogbmV4dDsKKyAgICB3aGlsZSAoIShuZXh0ID0gY3VycmVu dC0+bmV4dFNpYmxpbmcoKSkpIHsKKyAgICAgICAgY3VycmVudCA9IGN1cnJlbnQtPnBhcmVudCgp OworICAgICAgICBpZiAoIWN1cnJlbnQgfHwgY3VycmVudCA9PSBzdGF5V2l0aGluKQorICAgICAg ICAgICAgcmV0dXJuIDA7CiAgICAgfQotCi0gICAgcmV0dXJuIG87CisgICAgcmV0dXJuIG5leHQ7 CiB9CiAKIFJlbmRlck9iamVjdCogUmVuZGVyT2JqZWN0OjpwcmV2aW91c0luUHJlT3JkZXIoKSBj b25zdAo=