65086 2011-07-24 15:02:15 -0700 Fix AtomicString vs. String usage in accessibility code 2011-07-26 04:32:59 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 65172 1 paroga paroga darin webkit.review.bot oldest_to_newest 441494 0 paroga 2011-07-24 15:02:15 -0700 Fix AtomicString vs. String usage in accessibility code 441495 1 101835 paroga 2011-07-24 15:07:42 -0700 Created attachment 101835 Patch 441515 2 101835 darin 2011-07-24 17:38:13 -0700 Comment on attachment 101835 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101835&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1237 > - String idName = idVector[i]; > + AtomicString idName(idVector[i]); > Element* idElement = scope->getElementById(idName); There’s no value to having a local variable here. It should just be getElementById(idVector[i]). 441550 3 paroga 2011-07-24 20:57:20 -0700 (In reply to comment #2) > (From update of attachment 101835 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101835&action=review > > > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1237 > > - String idName = idVector[i]; > > + AtomicString idName(idVector[i]); > > Element* idElement = scope->getElementById(idName); > > There’s no value to having a local variable here. It should just be getElementById(idVector[i]). We need to make a AtomicString out of String, to make the explicit cast work (I should mention that in the ChangeLog... sorry). The alternative line would be getElementById(AtomicString(idVector[i])). An other option is to add a split operator which generates a list of AtomicString, but I don't see much value in this. 441885 4 darin 2011-07-25 14:53:44 -0700 (In reply to comment #3) > An other option is to add a split operator which generates a list of AtomicString, but I don't see much value in this. There’s a little bit of value in that. 441961 5 101835 webkit.review.bot 2011-07-25 16:12:56 -0700 Comment on attachment 101835 Patch Clearing flags on attachment: 101835 Committed r91718: <http://trac.webkit.org/changeset/91718> 441962 6 webkit.review.bot 2011-07-25 16:13:00 -0700 All reviewed patches have been landed. Closing bug. 101835 2011-07-24 15:07:42 -0700 2011-07-25 16:12:56 -0700 Patch bug-65086-20110725000740.patch text/plain 4113 paroga U3VidmVyc2lvbiBSZXZpc2lvbjogOTE2NTAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBjYTMzNTE5NzIxMjI2YmVj NDA3ZGY3MjA1NmRlNmRiNGExZDA5NjQ1Li4zOTJlYTMyMDM2NDMzYjJhMjhmM2FjNWMwMjNhMTQx ZDViNWMxZTJjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTEtMDctMjQgIFBhdHJp Y2sgR2Fuc3RlcmVyICA8cGFyb2dhQHdlYmtpdC5vcmc+CisKKyAgICAgICAgRml4IEF0b21pY1N0 cmluZyB2cy4gU3RyaW5nIHVzYWdlIGluIGFjY2Vzc2liaWxpdHkgY29kZQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjUwODYKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBVc2UgQXRvbWljU3RyaW5nIGluc3Rl YWQgb2YgU3RyaW5nIHdoZXJlIGFwcHJvcHJpYXRlCisgICAgICAgIHRvIGF2b2lkIGltcGxpY2l0 IHR5cGUgY29udmVyc2F0aW9ucy4KKyAgICAgICAgQWxzbyBmaXggYSBpbnZhbGlkIHJlZmVyZW5j ZSB0byBhIHRlbXBvcmFyeSBTdHJpbmcuCisKKyAgICAgICAgKiBhY2Nlc3NpYmlsaXR5L0FjY2Vz c2liaWxpdHlPYmplY3QuaDoKKyAgICAgICAgKFdlYkNvcmU6OkFjY2Vzc2liaWxpdHlPYmplY3Q6 OnNldEFjY2Vzc2libGVOYW1lKToKKyAgICAgICAgKiBhY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxp dHlSZW5kZXJPYmplY3QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6QWNjZXNzaWJpbGl0eVJlbmRl ck9iamVjdDo6ZWxlbWVudHNGcm9tQXR0cmlidXRlKToKKyAgICAgICAgKFdlYkNvcmU6OkFjY2Vz c2liaWxpdHlSZW5kZXJPYmplY3Q6OmFyaWFBY2Nlc3NpYmlsaXR5RGVzY3JpcHRpb24pOgorICAg ICAgICAoV2ViQ29yZTo6QWNjZXNzaWJpbGl0eVJlbmRlck9iamVjdDo6c2V0QWNjZXNzaWJsZU5h bWUpOgorICAgICAgICAqIGFjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eVJlbmRlck9iamVjdC5o OgorCiAyMDExLTA3LTI0ICBSb2IgQnVpcyAgPHJidWlzQHJpbS5jb20+CiAKICAgICAgICAgU1ZH IDx1c2U+IGVsZW1lbnQgaXMgbm90IHJlcG9zaXRpb25lZCB3aGVuIG1vdmVkIHRvIHg9MCB5PTAg dGhyb3VnaCBzY3JpcHQgCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5 L0FjY2Vzc2liaWxpdHlPYmplY3QuaCBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNj ZXNzaWJpbGl0eU9iamVjdC5oCmluZGV4IGUwMDMxNTNkNjVmNmMxYmEyZWFiN2IwZWM3Y2MxZmNk YzM1NjE1ZjQuLmJmNjZlNTI4ZTM4NTAzZDZjNGYwYmFlZjliYjBhNzFhNGNmOGIxYjEgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU9iamVjdC5o CisrKyBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU9iamVjdC5o CkBAIC0zNDIsNyArMzQyLDcgQEAgcHVibGljOgogICAgIHZpcnR1YWwgYm9vbCBjYW5TZXRFeHBh bmRlZEF0dHJpYnV0ZSgpIGNvbnN0IHsgcmV0dXJuIGZhbHNlOyB9CiAgICAgCiAgICAgLy8gQSBw cm9ncmFtbWF0aWMgd2F5IHRvIHNldCBhIG5hbWUgb24gYW4gQWNjZXNzaWJsZU9iamVjdC4KLSAg ICB2aXJ0dWFsIHZvaWQgc2V0QWNjZXNzaWJsZU5hbWUoU3RyaW5nJikgeyB9CisgICAgdmlydHVh bCB2b2lkIHNldEFjY2Vzc2libGVOYW1lKGNvbnN0IEF0b21pY1N0cmluZyYpIHsgfQogICAgIAog ICAgIHZpcnR1YWwgTm9kZSogbm9kZSgpIGNvbnN0IHsgcmV0dXJuIDA7IH0KICAgICB2aXJ0dWFs IFJlbmRlck9iamVjdCogcmVuZGVyZXIoKSBjb25zdCB7IHJldHVybiAwOyB9CmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3Qu Y3BwIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BY2Nlc3NpYmlsaXR5UmVuZGVyT2Jq ZWN0LmNwcAppbmRleCA4ZDY1OWZkNzhkZDk5YTE0NGZjMTg1NzE2ODIwYTQxNTAyYTI5NjFlLi4x MmMxNjdkYzExOTMyNmI1NWIwOGU1YmUwNjAyMDA3ZTUwNmYxZjA4IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3QuY3BwCisr KyBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eVJlbmRlck9iamVj dC5jcHAKQEAgLTEyMzMsNyArMTIzMyw3IEBAIHZvaWQgQWNjZXNzaWJpbGl0eVJlbmRlck9iamVj dDo6ZWxlbWVudHNGcm9tQXR0cmlidXRlKFZlY3RvcjxFbGVtZW50Kj4mIGVsZW1lbnRzCiAgICAg CiAgICAgdW5zaWduZWQgc2l6ZSA9IGlkVmVjdG9yLnNpemUoKTsKICAgICBmb3IgKHVuc2lnbmVk IGkgPSAwOyBpIDwgc2l6ZTsgKytpKSB7Ci0gICAgICAgIFN0cmluZyBpZE5hbWUgPSBpZFZlY3Rv cltpXTsKKyAgICAgICAgQXRvbWljU3RyaW5nIGlkTmFtZShpZFZlY3RvcltpXSk7CiAgICAgICAg IEVsZW1lbnQqIGlkRWxlbWVudCA9IHNjb3BlLT5nZXRFbGVtZW50QnlJZChpZE5hbWUpOwogICAg ICAgICBpZiAoaWRFbGVtZW50KQogICAgICAgICAgICAgZWxlbWVudHMuYXBwZW5kKGlkRWxlbWVu dCk7CkBAIC0xMzQxLDcgKzEzNDEsNyBAQCBTdHJpbmcgQWNjZXNzaWJpbGl0eVJlbmRlck9iamVj dDo6YXJpYURlc2NyaWJlZEJ5QXR0cmlidXRlKCkgY29uc3QKICAgICAKIFN0cmluZyBBY2Nlc3Np YmlsaXR5UmVuZGVyT2JqZWN0OjphcmlhQWNjZXNzaWJpbGl0eURlc2NyaXB0aW9uKCkgY29uc3QK IHsKLSAgICBjb25zdCBBdG9taWNTdHJpbmcmIGFyaWFMYWJlbGVkQnkgPSBhcmlhTGFiZWxlZEJ5 QXR0cmlidXRlKCk7CisgICAgU3RyaW5nIGFyaWFMYWJlbGVkQnkgPSBhcmlhTGFiZWxlZEJ5QXR0 cmlidXRlKCk7CiAgICAgaWYgKCFhcmlhTGFiZWxlZEJ5LmlzRW1wdHkoKSkKICAgICAgICAgcmV0 dXJuIGFyaWFMYWJlbGVkQnk7CiAKQEAgLTM2MTUsNyArMzYxNSw3IEBAIGNvbnN0IFN0cmluZyYg QWNjZXNzaWJpbGl0eVJlbmRlck9iamVjdDo6YWN0aW9uVmVyYigpIGNvbnN0CiAgICAgfQogfQog ICAgIAotdm9pZCBBY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0OjpzZXRBY2Nlc3NpYmxlTmFtZShT dHJpbmcmIG5hbWUpCit2b2lkIEFjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3Q6OnNldEFjY2Vzc2li bGVOYW1lKGNvbnN0IEF0b21pY1N0cmluZyYgbmFtZSkKIHsKICAgICAvLyBTZXR0aW5nIHRoZSBh Y2Nlc3NpYmxlIG5hbWUgY2FuIHN0b3JlIHRoZSB2YWx1ZSBpbiB0aGUgRE9NCiAgICAgaWYgKCFt X3JlbmRlcmVyKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BY2Nl c3NpYmlsaXR5UmVuZGVyT2JqZWN0LmggYi9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0Fj Y2Vzc2liaWxpdHlSZW5kZXJPYmplY3QuaAppbmRleCBkMTM2YjhhZDM0YTQxNTUzOWM5MTliMmMx MmRjNGI5OGEwMWY2Y2JjLi4zNjFkZWVlZmYwYjg2MWMzNTU2YjBmNWE0OTFmYTc1MTc5OGFmMDA0 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2liaWxpdHlS ZW5kZXJPYmplY3QuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FjY2Vzc2li aWxpdHlSZW5kZXJPYmplY3QuaApAQCAtMTA3LDcgKzEwNyw3IEBAIHB1YmxpYzoKICAgICB2aXJ0 dWFsIGJvb2wgY2FuU2V0VmFsdWVBdHRyaWJ1dGUoKSBjb25zdDsKICAgICB2aXJ0dWFsIGJvb2wg Y2FuU2V0RXhwYW5kZWRBdHRyaWJ1dGUoKSBjb25zdDsKIAotICAgIHZpcnR1YWwgdm9pZCBzZXRB Y2Nlc3NpYmxlTmFtZShTdHJpbmcmKTsKKyAgICB2aXJ0dWFsIHZvaWQgc2V0QWNjZXNzaWJsZU5h bWUoY29uc3QgQXRvbWljU3RyaW5nJik7CiAgICAgCiAgICAgLy8gUHJvdmlkZXMgY29tbW9uIGxv Z2ljIHVzZWQgYnkgYWxsIGVsZW1lbnRzIHdoZW4gZGV0ZXJtaW5pbmcgaXNJZ25vcmVkLgogICAg IEFjY2Vzc2liaWxpdHlPYmplY3RJbmNsdXNpb24gYWNjZXNzaWJpbGl0eUlzSWdub3JlZEJhc2Uo KSBjb25zdDsK