63368 2011-06-24 17:31:59 -0700 RenderTextFragment should have regular children instead of magical first-letter child 2013-05-08 06:53:05 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 58911 1 rniwa webkit-unassigned antaryami.pandia enrica eric hyatt igor.oliveira inferno leviw mitz simon.fraser tkent xji oldest_to_newest 427259 0 rniwa 2011-06-24 17:31:59 -0700 There are numerous bugs and security vulnerabilities due to first-letter / RenderTextFragment being so exotic to the rest of render objects. We should make RenderTextFragment act like the rest of rendering objects. Namely, it should own regular children instead of holding a special pointer for first-letter. This will fix almost all crashes and bugs related to first-letter in WebCore/editing. 427266 1 leviw 2011-06-24 17:54:47 -0700 Yesssss! This was the conclusion I drew in my own frustration dealing with first-letter and editing. Seeing these problems continue to crop up really drives the issue home. I'm happy to work on this. 427294 2 inferno 2011-06-24 20:22:25 -0700 Thanks a lot Levi and Ryosuke. 427458 3 rniwa 2011-06-25 16:21:47 -0700 There are 2 possible approaches: 1. Keep RenderTextFragment inherit from RenderText. In this approach, RenderTextFragment will act like RenderText and we can work-around most of issues in editing by adding a method to return the entire text instead "everything but first letter". The problem with this approach is that renderes of inline text boxes will still be that of internal RenderText unless we wrap renderer() as well. 2. Make RenderTextFragment a render inline. In this approach, we'll reimplement first-letter and act like anonymous node that contains two RenderText. There are a lot of uncertainlies and risks but the biggest problem is that Text DOM nodes's renderer() will no longer return RenderText. It might complicate many logics in editing and accessibility in addition to rendering, hit testing, and caret rendering, etc... 428110 4 leviw 2011-06-27 11:15:45 -0700 I talked with Ryosuke about this a bit. In my opinion, the first approach is viable and worthy of a proof of concept to make sure it's not out of the scope we think it is. I worry option 2 would be too invasive, kind of like getting the current first-letter implementation to work properly :) 648904 5 147509 antaryami.pandia 2012-06-14 00:59:45 -0700 Created attachment 147509 WIP patch. Basic WIP patch. 147509 2012-06-14 00:59:45 -0700 2012-06-14 00:59:45 -0700 WIP patch. 63368-wip.diff text/plain 6993 antaryami.pandia SW5kZXg6IFNvdXJjZS9XZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV2ViQ29yZS9lZGl0aW5nL1RleHRJdGVyYXRvci5jcHAJKHJldmlzaW9uIDEx OTk3MSkKKysrIFNvdXJjZS9XZWJDb3JlL2VkaXRpbmcvVGV4dEl0ZXJhdG9yLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtNjMzLDE5ICs2MzMsNyBAQCBzdGF0aWMgaW5saW5lIFJlbmRlclRleHQqIGZp cnN0UmVuZGVyVGV4CiAKIHZvaWQgVGV4dEl0ZXJhdG9yOjpoYW5kbGVUZXh0Tm9kZUZpcnN0TGV0 dGVyKFJlbmRlclRleHRGcmFnbWVudCogcmVuZGVyZXIpCiB7Ci0gICAgaWYgKHJlbmRlcmVyLT5m aXJzdExldHRlcigpKSB7Ci0gICAgICAgIFJlbmRlck9iamVjdCogciA9IHJlbmRlcmVyLT5maXJz dExldHRlcigpOwotICAgICAgICBpZiAoci0+c3R5bGUoKS0+dmlzaWJpbGl0eSgpICE9IFZJU0lC TEUgJiYgIW1faWdub3Jlc1N0eWxlVmlzaWJpbGl0eSkKLSAgICAgICAgICAgIHJldHVybjsKLSAg ICAgICAgaWYgKFJlbmRlclRleHQqIGZpcnN0TGV0dGVyID0gZmlyc3RSZW5kZXJUZXh0SW5GaXJz dExldHRlcihyKSkgewotICAgICAgICAgICAgbV9oYW5kbGVkRmlyc3RMZXR0ZXIgPSB0cnVlOwot ICAgICAgICAgICAgbV9yZW1haW5pbmdUZXh0Qm94ID0gbV90ZXh0Qm94OwotICAgICAgICAgICAg bV90ZXh0Qm94ID0gZmlyc3RMZXR0ZXItPmZpcnN0VGV4dEJveCgpOwotICAgICAgICAgICAgbV9z b3J0ZWRUZXh0Qm94ZXMuY2xlYXIoKTsKLSAgICAgICAgICAgIG1fZmlyc3RMZXR0ZXJUZXh0ID0g Zmlyc3RMZXR0ZXI7Ci0gICAgICAgIH0KLSAgICB9Ci0gICAgbV9oYW5kbGVkRmlyc3RMZXR0ZXIg PSB0cnVlOworICAgIC8vIFRPRE86IEltcGxlbWVudCB0aGlzLgogfQogCiBib29sIFRleHRJdGVy YXRvcjo6aGFuZGxlUmVwbGFjZWRFbGVtZW50KCkKQEAgLTcwMSwxMyArNjg5LDcgQEAgYm9vbCBU ZXh0SXRlcmF0b3I6OmhhbmRsZVJlcGxhY2VkRWxlbWVudAogCiBib29sIFRleHRJdGVyYXRvcjo6 aGFzVmlzaWJsZVRleHROb2RlKFJlbmRlclRleHQqIHJlbmRlcmVyKQogewotICAgIGlmIChyZW5k ZXJlci0+c3R5bGUoKS0+dmlzaWJpbGl0eSgpID09IFZJU0lCTEUpCi0gICAgICAgIHJldHVybiB0 cnVlOwotICAgIGlmIChyZW5kZXJlci0+aXNUZXh0RnJhZ21lbnQoKSkgewotICAgICAgICBSZW5k ZXJUZXh0RnJhZ21lbnQqIGZyYWdtZW50ID0gc3RhdGljX2Nhc3Q8UmVuZGVyVGV4dEZyYWdtZW50 Kj4ocmVuZGVyZXIpOwotICAgICAgICBpZiAoZnJhZ21lbnQtPmZpcnN0TGV0dGVyKCkgJiYgZnJh Z21lbnQtPmZpcnN0TGV0dGVyKCktPnN0eWxlKCktPnZpc2liaWxpdHkoKSA9PSBWSVNJQkxFKQot ICAgICAgICAgICAgcmV0dXJuIHRydWU7Ci0gICAgfQorICAgIC8vIFRPRE86IEltcGxlbWVudCB0 aGlzLgogICAgIHJldHVybiBmYWxzZTsKIH0KIApAQCAtMTMwMiw3ICsxMjg0LDEwIEBAIFJlbmRl clRleHQqIFNpbXBsaWZpZWRCYWNrd2FyZHNUZXh0SXRlcmEKIAogICAgIG1fc2hvdWxkSGFuZGxl Rmlyc3RMZXR0ZXIgPSBmYWxzZTsKICAgICBvZmZzZXRJbk5vZGUgPSAwOwotICAgIHJldHVybiBm aXJzdFJlbmRlclRleHRJbkZpcnN0TGV0dGVyKGZyYWdtZW50LT5maXJzdExldHRlcigpKTsKKwor ICAgIC8vIFRPRE86IEhhbmRsZSB0aGlzIHNjZW5lcmlvLgorICAgIC8vcmV0dXJuIGZpcnN0UmVu ZGVyVGV4dEluRmlyc3RMZXR0ZXIoZnJhZ21lbnQtPmZpcnN0TGV0dGVyKCkpOworICAgIHJldHVy biAwOwogfQogCiBib29sIFNpbXBsaWZpZWRCYWNrd2FyZHNUZXh0SXRlcmF0b3I6OmhhbmRsZVJl cGxhY2VkRWxlbWVudCgpCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxv Y2suY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJCbG9j ay5jcHAJKHJldmlzaW9uIDExOTk3MSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5k ZXJCbG9jay5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTYwNDIsNyArNjA0Miw2IEBAIHZvaWQgUmVu ZGVyQmxvY2s6OnVwZGF0ZUZpcnN0TGV0dGVyU3R5bGUKICAgICAgICAgaWYgKHJlbWFpbmluZ1Rl eHQpIHsKICAgICAgICAgICAgIEFTU0VSVChyZW1haW5pbmdUZXh0LT5pc0Fub255bW91cygpIHx8 IHJlbWFpbmluZ1RleHQtPm5vZGUoKS0+cmVuZGVyZXIoKSA9PSByZW1haW5pbmdUZXh0KTsKICAg ICAgICAgICAgIC8vIFJlcGxhY2UgdGhlIG9sZCByZW5kZXJlciB3aXRoIHRoZSBuZXcgb25lLgot ICAgICAgICAgICAgcmVtYWluaW5nVGV4dC0+c2V0Rmlyc3RMZXR0ZXIobmV3Rmlyc3RMZXR0ZXIp OwogICAgICAgICAgICAgdG9SZW5kZXJCb3hNb2RlbE9iamVjdChuZXdGaXJzdExldHRlciktPnNl dEZpcnN0TGV0dGVyUmVtYWluaW5nVGV4dChyZW1haW5pbmdUZXh0KTsKICAgICAgICAgfQogICAg ICAgICAvLyBUbyBwcmV2ZW50IHJlbW92YWwgb2Ygc2luZ2xlIGFub255bW91cyBibG9jayBpbiBS ZW5kZXJCbG9jazo6cmVtb3ZlQ2hpbGQgYW5kIGNhdXNpbmcKQEAgLTYxMTIsMTIgKzYxMTEsMTEg QEAgdm9pZCBSZW5kZXJCbG9jazo6Y3JlYXRlRmlyc3RMZXR0ZXJSZW5kZQogCiAgICAgICAgIGZp cnN0TGV0dGVyQ29udGFpbmVyLT5hZGRDaGlsZChyZW1haW5pbmdUZXh0LCB0ZXh0T2JqKTsKICAg ICAgICAgZmlyc3RMZXR0ZXJDb250YWluZXItPnJlbW92ZUNoaWxkKHRleHRPYmopOwotICAgICAg ICByZW1haW5pbmdUZXh0LT5zZXRGaXJzdExldHRlcihmaXJzdExldHRlcik7CiAgICAgICAgIHRv UmVuZGVyQm94TW9kZWxPYmplY3QoZmlyc3RMZXR0ZXIpLT5zZXRGaXJzdExldHRlclJlbWFpbmlu Z1RleHQocmVtYWluaW5nVGV4dCk7CiAgICAgICAgIAogICAgICAgICAvLyBjb25zdHJ1Y3QgdGV4 dCBmcmFnbWVudCBmb3IgdGhlIGZpcnN0IGxldHRlcgogICAgICAgICBSZW5kZXJUZXh0RnJhZ21l bnQqIGxldHRlciA9IAotICAgICAgICAgICAgbmV3IChyZW5kZXJBcmVuYSgpKSBSZW5kZXJUZXh0 RnJhZ21lbnQocmVtYWluaW5nVGV4dC0+bm9kZSgpID8gcmVtYWluaW5nVGV4dC0+bm9kZSgpIDog cmVtYWluaW5nVGV4dC0+ZG9jdW1lbnQoKSwgb2xkVGV4dC5nZXQoKSwgMCwgbGVuZ3RoKTsKKyAg ICAgICAgICAgIG5ldyAocmVuZGVyQXJlbmEoKSkgUmVuZGVyVGV4dEZyYWdtZW50KHJlbWFpbmlu Z1RleHQtPm5vZGUoKSA/IHJlbWFpbmluZ1RleHQtPm5vZGUoKSA6IHJlbWFpbmluZ1RleHQtPmRv Y3VtZW50KCksIG9sZFRleHQuZ2V0KCksIDAsIGxlbmd0aCwgdHJ1ZSk7CiAgICAgICAgIGxldHRl ci0+c2V0U3R5bGUocHNldWRvU3R5bGUpOwogICAgICAgICBmaXJzdExldHRlci0+YWRkQ2hpbGQo bGV0dGVyKTsKIApJbmRleDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRGcmFn bWVudC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRl eHRGcmFnbWVudC5jcHAJKHJldmlzaW9uIDExOTk3MSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJUZXh0RnJhZ21lbnQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yOCwxMSArMjgs MTEgQEAKIAogbmFtZXNwYWNlIFdlYkNvcmUgewogCi1SZW5kZXJUZXh0RnJhZ21lbnQ6OlJlbmRl clRleHRGcmFnbWVudChOb2RlKiBub2RlLCBTdHJpbmdJbXBsKiBzdHIsIGludCBzdGFydE9mZnNl dCwgaW50IGxlbmd0aCkKK1JlbmRlclRleHRGcmFnbWVudDo6UmVuZGVyVGV4dEZyYWdtZW50KE5v ZGUqIG5vZGUsIFN0cmluZ0ltcGwqIHN0ciwgaW50IHN0YXJ0T2Zmc2V0LCBpbnQgbGVuZ3RoLCBi b29sIGlzRmlyc3RMZXR0ZXJUZXh0RnJhZ21lbnQpCiAgICAgOiBSZW5kZXJUZXh0KG5vZGUsIHN0 ciA/IHN0ci0+c3Vic3RyaW5nKHN0YXJ0T2Zmc2V0LCBsZW5ndGgpIDogUGFzc1JlZlB0cjxTdHJp bmdJbXBsPigwKSkKICAgICAsIG1fc3RhcnQoc3RhcnRPZmZzZXQpCiAgICAgLCBtX2VuZChsZW5n dGgpCi0gICAgLCBtX2ZpcnN0TGV0dGVyKDApCisgICAgLCBtX2lzRmlyc3RMZXR0ZXJUZXh0RnJh Z21lbnQoaXNGaXJzdExldHRlclRleHRGcmFnbWVudCkKIHsKIH0KIApAQCAtNDEsNyArNDEsNyBA QCBSZW5kZXJUZXh0RnJhZ21lbnQ6OlJlbmRlclRleHRGcmFnbWVudChOCiAgICAgLCBtX3N0YXJ0 KDApCiAgICAgLCBtX2VuZChzdHIgPyBzdHItPmxlbmd0aCgpIDogMCkKICAgICAsIG1fY29udGVu dFN0cmluZyhzdHIpCi0gICAgLCBtX2ZpcnN0TGV0dGVyKDApCisgICAgLCBtX2lzRmlyc3RMZXR0 ZXJUZXh0RnJhZ21lbnQoZmFsc2UpCiB7CiB9CiAKQEAgLTcwLDggKzcwLDYgQEAgdm9pZCBSZW5k ZXJUZXh0RnJhZ21lbnQ6OnN0eWxlRGlkQ2hhbmdlKAogCiB2b2lkIFJlbmRlclRleHRGcmFnbWVu dDo6d2lsbEJlRGVzdHJveWVkKCkKIHsKLSAgICBpZiAobV9maXJzdExldHRlcikKLSAgICAgICAg bV9maXJzdExldHRlci0+ZGVzdHJveSgpOwogICAgIFJlbmRlclRleHQ6OndpbGxCZURlc3Ryb3ll ZCgpOwogfQogCkBAIC04MSwxNSArNzksNiBAQCB2b2lkIFJlbmRlclRleHRGcmFnbWVudDo6c2V0 VGV4dChQYXNzUmVmCiAKICAgICBtX3N0YXJ0ID0gMDsKICAgICBtX2VuZCA9IHRleHRMZW5ndGgo KTsKLSAgICBpZiAobV9maXJzdExldHRlcikgewotICAgICAgICBBU1NFUlQoIW1fY29udGVudFN0 cmluZyk7Ci0gICAgICAgIG1fZmlyc3RMZXR0ZXItPmRlc3Ryb3koKTsKLSAgICAgICAgbV9maXJz dExldHRlciA9IDA7Ci0gICAgICAgIGlmIChOb2RlKiB0ID0gbm9kZSgpKSB7Ci0gICAgICAgICAg ICBBU1NFUlQoIXQtPnJlbmRlcmVyKCkpOwotICAgICAgICAgICAgdC0+c2V0UmVuZGVyZXIodGhp cyk7Ci0gICAgICAgIH0KLSAgICB9CiB9CiAKIHZvaWQgUmVuZGVyVGV4dEZyYWdtZW50Ojp0cmFu c2Zvcm1UZXh0KCkKQEAgLTExMyw5ICsxMDIsMTAgQEAgVUNoYXIgUmVuZGVyVGV4dEZyYWdtZW50 OjpwcmV2aW91c0NoYXJhYwogCiBSZW5kZXJCbG9jayogUmVuZGVyVGV4dEZyYWdtZW50OjpibG9j a0ZvckFjY29tcGFueWluZ0ZpcnN0TGV0dGVyKCkgY29uc3QKIHsKLSAgICBpZiAoIW1fZmlyc3RM ZXR0ZXIpCisgICAgaWYgKG1faXNGaXJzdExldHRlclRleHRGcmFnbWVudCkKICAgICAgICAgcmV0 dXJuIDA7Ci0gICAgZm9yIChSZW5kZXJPYmplY3QqIGJsb2NrID0gbV9maXJzdExldHRlci0+cGFy ZW50KCk7IGJsb2NrOyBibG9jayA9IGJsb2NrLT5wYXJlbnQoKSkgeworCisgICAgZm9yIChSZW5k ZXJPYmplY3QqIGJsb2NrID0gcGFyZW50KCk7IGJsb2NrOyBibG9jayA9IGJsb2NrLT5wYXJlbnQo KSkgewogICAgICAgICBpZiAoYmxvY2stPnN0eWxlKCktPmhhc1BzZXVkb1N0eWxlKEZJUlNUX0xF VFRFUikgJiYgYmxvY2stPmNhbkhhdmVDaGlsZHJlbigpICYmIGJsb2NrLT5pc1JlbmRlckJsb2Nr KCkpCiAgICAgICAgICAgICByZXR1cm4gdG9SZW5kZXJCbG9jayhibG9jayk7CiAgICAgfQpJbmRl eDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRGcmFnbWVudC5oCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUZXh0RnJhZ21lbnQuaAkocmV2 aXNpb24gMTE5OTcxKQorKysgU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRGcmFn bWVudC5oCSh3b3JraW5nIGNvcHkpCkBAIC0zMyw3ICszMyw3IEBAIG5hbWVzcGFjZSBXZWJDb3Jl IHsKIC8vIHRoZSBvcmlnaW5hbCB1bmFsdGVyZWQgc3RyaW5nIGZyb20gb3VyIGNvcnJlc3BvbmRp bmcgRE9NIG5vZGUuCiBjbGFzcyBSZW5kZXJUZXh0RnJhZ21lbnQgOiBwdWJsaWMgUmVuZGVyVGV4 dCB7CiBwdWJsaWM6Ci0gICAgUmVuZGVyVGV4dEZyYWdtZW50KE5vZGUqLCBTdHJpbmdJbXBsKiwg aW50IHN0YXJ0T2Zmc2V0LCBpbnQgbGVuZ3RoKTsKKyAgICBSZW5kZXJUZXh0RnJhZ21lbnQoTm9k ZSosIFN0cmluZ0ltcGwqLCBpbnQgc3RhcnRPZmZzZXQsIGludCBsZW5ndGgsIGJvb2wgaXNGaXJz dExldHRlclRleHRGcmFnbWVudCA9IGZhbHNlKTsKICAgICBSZW5kZXJUZXh0RnJhZ21lbnQoTm9k ZSosIFN0cmluZ0ltcGwqKTsKICAgICB2aXJ0dWFsIH5SZW5kZXJUZXh0RnJhZ21lbnQoKTsKIApA QCAtNDQsOSArNDQsNiBAQCBwdWJsaWM6CiAgICAgdW5zaWduZWQgc3RhcnQoKSBjb25zdCB7IHJl dHVybiBtX3N0YXJ0OyB9CiAgICAgdW5zaWduZWQgZW5kKCkgY29uc3QgeyByZXR1cm4gbV9lbmQ7 IH0KIAotICAgIFJlbmRlck9iamVjdCogZmlyc3RMZXR0ZXIoKSBjb25zdCB7IHJldHVybiBtX2Zp cnN0TGV0dGVyOyB9Ci0gICAgdm9pZCBzZXRGaXJzdExldHRlcihSZW5kZXJPYmplY3QqIGZpcnN0 TGV0dGVyKSB7IG1fZmlyc3RMZXR0ZXIgPSBmaXJzdExldHRlcjsgfQotCiAgICAgU3RyaW5nSW1w bCogY29udGVudFN0cmluZygpIGNvbnN0IHsgcmV0dXJuIG1fY29udGVudFN0cmluZy5nZXQoKTsg fQogICAgIHZpcnR1YWwgUGFzc1JlZlB0cjxTdHJpbmdJbXBsPiBvcmlnaW5hbFRleHQoKSBjb25z dDsKIApAQCAtNjYsNyArNjMsNyBAQCBwcml2YXRlOgogICAgIHVuc2lnbmVkIG1fc3RhcnQ7CiAg ICAgdW5zaWduZWQgbV9lbmQ7CiAgICAgUmVmUHRyPFN0cmluZ0ltcGw+IG1fY29udGVudFN0cmlu ZzsKLSAgICBSZW5kZXJPYmplY3QqIG1fZmlyc3RMZXR0ZXI7CisgICAgYm9vbCBtX2lzRmlyc3RM ZXR0ZXJUZXh0RnJhZ21lbnQ7CiB9OwogCiBpbmxpbmUgUmVuZGVyVGV4dEZyYWdtZW50KiB0b1Jl bmRlclRleHRGcmFnbWVudChSZW5kZXJPYmplY3QqIG9iamVjdCkK