106533 2013-01-09 23:39:16 -0800 [Refactoring] HTMLTextFormControlElement should use shadowHost instead of shadowAncestorNode 2013-01-17 20:17:23 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 91821 107089 1 shinyak shinyak mifenton ojan.autocc tkent webkit.review.bot oldest_to_newest 804109 0 shinyak 2013-01-09 23:39:16 -0800 shadowAncestorNode() is deprecated. We would like to use shadowHost. 804114 1 182076 shinyak 2013-01-09 23:53:47 -0800 Created attachment 182076 Patch 807166 2 182076 tkent 2013-01-15 03:39:08 -0800 Comment on attachment 182076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182076&action=review ok > Source/WebCore/html/HTMLTextFormControlElement.cpp:647 > + return ancestor ? toTextFormControl(ancestor) : 0; Not your fault, but this code looks dangerous. Who ensures that the position points shadow nodes in text input or textarea? 809034 3 shinyak 2013-01-16 20:43:53 -0800 I'll land this patch anyway, and I'll file another bug to address the issue. 809039 4 shinyak 2013-01-16 20:45:48 -0800 https://bugs.webkit.org/show_bug.cgi?id=107089 809051 5 182076 webkit.review.bot 2013-01-16 21:14:26 -0800 Comment on attachment 182076 Patch Clearing flags on attachment: 182076 Committed r139962: <http://trac.webkit.org/changeset/139962> 809052 6 webkit.review.bot 2013-01-16 21:14:29 -0800 All reviewed patches have been landed. Closing bug. 809399 7 182076 darin 2013-01-17 09:42:35 -0800 Comment on attachment 182076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182076&action=review > Source/WebCore/html/HTMLTextFormControlElement.cpp:646 > + Node* ancestor = container->shadowHost(); The ancestor variable should be of type Element*, not Node*. >> Source/WebCore/html/HTMLTextFormControlElement.cpp:647 >> + return ancestor ? toTextFormControl(ancestor) : 0; > > Not your fault, but this code looks dangerous. Who ensures that the position points shadow nodes in text input or textarea? Good point, this can be a bad cast! We need a type check. 810159 8 182076 shinyak 2013-01-17 20:17:23 -0800 Comment on attachment 182076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182076&action=review >>> Source/WebCore/html/HTMLTextFormControlElement.cpp:647 >>> + return ancestor ? toTextFormControl(ancestor) : 0; >> >> Not your fault, but this code looks dangerous. Who ensures that the position points shadow nodes in text input or textarea? > > Good point, this can be a bad cast! We need a type check. toTextFormControl() returns 0 if it's not TextFormControl. So this would be safe, but I think it does not align our recent code convention. Maybe refactoring is desired. 182076 2013-01-09 23:53:47 -0800 2013-01-17 20:17:23 -0800 Patch bug-106533-20130110165050.patch text/plain 3107 shinyak U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5MjY2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWUxM2RkNWNmMjJjYzYz NThmZDg3OTVlMTcwOGI5NWYwZmIxMmIyOS4uOTg2ZWI4ZGM5ZTA0NGI3YmY1M2FlMjFhNjU3NDhk ZGE3OThmMmRhNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEzLTAxLTA5ICBTaGlu eWEgS2F3YW5ha2EgIDxzaGlueWFrQGNocm9taXVtLm9yZz4KKworICAgICAgICBbUmVmYWN0b3Jp bmddIEhUTUxUZXh0Rm9ybUNvbnRyb2xFbGVtZW50IHNob3VsZCB1c2Ugc2hhZG93SG9zdCBpbnN0 ZWFkIG9mIHNoYWRvd0FuY2VzdG9yTm9kZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NTMzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgU2luY2UgTm9kZTo6c2hhZG93QW5jZXN0b3JOb2RlKCkgaXMgZGVw cmVjYXRlZCwgd2Ugd291bGQgbGlrZSB0byB1c2UgTm9kZTo6c2hhZG93SG9zdCgpLgorCisgICAg ICAgIE5vIG5ldyB0ZXN0cywgc2ltcGxlIHJlZmFjdG9yaW5nLgorCisgICAgICAgICogaHRtbC9I VE1MVGV4dEZvcm1Db250cm9sRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MVGV4 dEZvcm1Db250cm9sRWxlbWVudDo6c2V0U2VsZWN0aW9uUmFuZ2UpOiBIZXJlLCBzdGFydFBvc2l0 aW9uIGFuZCBlbmRQb3NpdGlvbiBpcyBpbiBTaGFkb3dET00sCisgICAgICAgIGNvbnZlcnRpbmcg c2hhZG93QW5jZXN0b3JOb2RlIHRvIHNoYWRvd0hvc3QgaXMgc2FmZS4KKyAgICAgICAgKFdlYkNv cmU6OmVuY2xvc2luZ1RleHRGb3JtQ29udHJvbCk6IEFjdHVhbGx5IEFTU0VSVCBpbiB0aGlzIG1l dGhvZCBpcyB3cm9uZyBzaW5jZSBzaGFkb3dBbmNlc3Rvck5vZGUoKSBkb2VzIG5vdAorICAgICAg ICByZXR1cm4gTlVMTC4gVGhpcyBzaG91bGQgYmUgc2hhZG93SG9zdCgpLgorCiAyMDEzLTAxLTA5 ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwg Zml4IGJ1aWxkIGFmdGVyIGh0dHA6Ly90cmFjLndlYmtpdC5vcmcvY2hhbmdlc2V0LzEzOTI2Mgpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVGV4dEZvcm1Db250cm9sRWxlbWVu dC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxUZXh0Rm9ybUNvbnRyb2xFbGVtZW50LmNw cAppbmRleCA1ODZjNTdiYjU5ZTQxNjI4OTgzMTIzY2Y0ZTY0OTVmZDJiMDIyMDk2Li44ODExNmE4 YjE3NzcwZDUyNTgzZDliM2IxN2JjY2QxZWQzZmJhZTM0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9odG1sL0hUTUxUZXh0Rm9ybUNvbnRyb2xFbGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9odG1sL0hUTUxUZXh0Rm9ybUNvbnRyb2xFbGVtZW50LmNwcApAQCAtMzMwLDggKzMzMCw4 IEBAIHZvaWQgSFRNTFRleHRGb3JtQ29udHJvbEVsZW1lbnQ6OnNldFNlbGVjdGlvblJhbmdlKGlu dCBzdGFydCwgaW50IGVuZCwgVGV4dEZpZWxkCiAgICAgLy8gc3RhcnRQb3NpdGlvbiBhbmQgZW5k UG9zaXRpb24gY2FuIGJlIG51bGwgcG9zaXRpb24gZm9yIGV4YW1wbGUgd2hlbgogICAgIC8vICIt d2Via2l0LXVzZXItc2VsZWN0OiBub25lIiBzdHlsZSBhdHRyaWJ1dGUgaXMgc3BlY2lmaWVkLgog ICAgIGlmIChzdGFydFBvc2l0aW9uLmlzTm90TnVsbCgpICYmIGVuZFBvc2l0aW9uLmlzTm90TnVs bCgpKSB7Ci0gICAgICAgIEFTU0VSVChzdGFydFBvc2l0aW9uLmRlZXBFcXVpdmFsZW50KCkuZGVw cmVjYXRlZE5vZGUoKS0+c2hhZG93QW5jZXN0b3JOb2RlKCkgPT0gdGhpcwotICAgICAgICAgICAg JiYgZW5kUG9zaXRpb24uZGVlcEVxdWl2YWxlbnQoKS5kZXByZWNhdGVkTm9kZSgpLT5zaGFkb3dB bmNlc3Rvck5vZGUoKSA9PSB0aGlzKTsKKyAgICAgICAgQVNTRVJUKHN0YXJ0UG9zaXRpb24uZGVl cEVxdWl2YWxlbnQoKS5kZXByZWNhdGVkTm9kZSgpLT5zaGFkb3dIb3N0KCkgPT0gdGhpcworICAg ICAgICAgICAgJiYgZW5kUG9zaXRpb24uZGVlcEVxdWl2YWxlbnQoKS5kZXByZWNhdGVkTm9kZSgp LT5zaGFkb3dIb3N0KCkgPT0gdGhpcyk7CiAgICAgfQogICAgIFZpc2libGVTZWxlY3Rpb24gbmV3 U2VsZWN0aW9uOwogICAgIGlmIChkaXJlY3Rpb24gPT0gU2VsZWN0aW9uSGFzQmFja3dhcmREaXJl Y3Rpb24pCkBAIC02MzksMTIgKzYzOSwxMiBAQCBTdHJpbmcgSFRNTFRleHRGb3JtQ29udHJvbEVs ZW1lbnQ6OnZhbHVlV2l0aEhhcmRMaW5lQnJlYWtzKCkgY29uc3QKIEhUTUxUZXh0Rm9ybUNvbnRy b2xFbGVtZW50KiBlbmNsb3NpbmdUZXh0Rm9ybUNvbnRyb2woY29uc3QgUG9zaXRpb24mIHBvc2l0 aW9uKQogewogICAgIEFTU0VSVChwb3NpdGlvbi5pc051bGwoKSB8fCBwb3NpdGlvbi5hbmNob3JU eXBlKCkgPT0gUG9zaXRpb246OlBvc2l0aW9uSXNPZmZzZXRJbkFuY2hvcgotICAgICAgICAgICB8 fCBwb3NpdGlvbi5jb250YWluZXJOb2RlKCkgfHwgIXBvc2l0aW9uLmFuY2hvck5vZGUoKS0+c2hh ZG93QW5jZXN0b3JOb2RlKCkpOworICAgICAgICB8fCBwb3NpdGlvbi5jb250YWluZXJOb2RlKCkg fHwgIXBvc2l0aW9uLmFuY2hvck5vZGUoKS0+c2hhZG93SG9zdCgpKTsKICAgICBOb2RlKiBjb250 YWluZXIgPSBwb3NpdGlvbi5jb250YWluZXJOb2RlKCk7CiAgICAgaWYgKCFjb250YWluZXIpCiAg ICAgICAgIHJldHVybiAwOwotICAgIE5vZGUqIGFuY2VzdG9yID0gY29udGFpbmVyLT5zaGFkb3dB bmNlc3Rvck5vZGUoKTsKLSAgICByZXR1cm4gYW5jZXN0b3IgIT0gY29udGFpbmVyID8gdG9UZXh0 Rm9ybUNvbnRyb2woYW5jZXN0b3IpIDogMDsKKyAgICBOb2RlKiBhbmNlc3RvciA9IGNvbnRhaW5l ci0+c2hhZG93SG9zdCgpOworICAgIHJldHVybiBhbmNlc3RvciA/IHRvVGV4dEZvcm1Db250cm9s KGFuY2VzdG9yKSA6IDA7CiB9CiAKIHN0YXRpYyBjb25zdCBFbGVtZW50KiBwYXJlbnRIVE1MRWxl bWVudChjb25zdCBFbGVtZW50KiBlbGVtZW50KQo=