85081 2012-04-27 10:55:57 -0700 accessibility/misspelled-attributed-string.html test sometimes throws exceptions 2012-05-03 10:06:40 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cfleizach cfleizach simon.fraser oldest_to_newest 611676 0 cfleizach 2012-04-27 10:55:57 -0700 It seems that sometimes results from the spell checker can return a range that is outside the bounds of what we've expected We should put more range checking into our attributed string methods 2012-04-19 11:28:28.275 WebProcess[4804:403] *** Terminating app due to uncaught exception 'NSRangeException', reason: 'NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds' *** First throw call stack: ( 0 CoreFoundation 0x00007fff9a5e4fc6 __exceptionPreprocess + 198 1 libobjc.A.dylib 0x00007fff8e343d5e objc_exception_throw + 43 2 CoreFoundation 0x00007fff9a5e4dfa +[NSException raise:format:arguments:] + 106 3 CoreFoundation 0x00007fff9a5e4d84 +[NSException raise:format:] + 116 4 Foundation 0x00007fff951a228d -[NSRLEArray objectAtIndex:effectiveRange:] + 117 5 Foundation 0x00007fff951a49b1 -[NSConcreteMutableAttributedString addAttribute:value:range:] + 166 6 WebCore 0x0000000108d9a519 _ZL28AXAttributedStringAppendTextP25NSMutableAttributedStringPN7WebCore4NodeEPKti + 2393 7 WebCore 0x0000000108d9966b -[WebAccessibilityObjectWrapper doAXAttributedStringForTextMarkerRange:] + 1387 8 WebCore 0x0000000108da490d -[WebAccessibilityObjectWrapper doAXAttributedStringForRange:] + 205 9 WebCore 0x0000000108da9c37 -[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:] + 19975 611678 1 139230 cfleizach 2012-04-27 10:57:04 -0700 Created attachment 139230 patch 612055 2 139230 darin 2012-04-27 18:07:27 -0700 Comment on attachment 139230 patch If the ranges can be “wrong” then we might want to do a more complete test than just testing is the end of the range is past the end of the string. For example, if a location is large, then adding the length to it might overflow, so NSMaxRange could be less than the string length. To handle that case too, we'd also want to check if the location is > [attString length]. A good way to do that would be a helper function named isValidRange or something like that. But if the type of wrong ranges we are getting are guaranteed to not have any overflow, then this patch might be OK. However, I think a “why” comment explaining the reason for these checks would be good. 614666 3 cfleizach 2012-05-02 14:28:12 -0700 (In reply to comment #2) > (From update of attachment 139230 [details]) > If the ranges can be “wrong” then we might want to do a more complete test than just testing is the end of the range is past the end of the string. For example, if a location is large, then adding the length to it might overflow, so NSMaxRange could be less than the string length. To handle that case too, we'd also want to check if the location is > [attString length]. A good way to do that would be a helper function named isValidRange or something like that. > > But if the type of wrong ranges we are getting are guaranteed to not have any overflow, then this patch might be OK. > We should also check against overflow. good idea. will do so > However, I think a “why” comment explaining the reason for these checks would be good. Yep 615371 4 cfleizach 2012-05-03 10:06:40 -0700 http://trac.webkit.org/changeset/115985 139230 2012-04-27 10:57:04 -0700 2012-04-27 18:07:27 -0700 patch patch.txt text/plain 3816 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDExNTQ0OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA0LTI3ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgYWNjZXNzaWJpbGl0eS9t aXNzcGVsbGVkLWF0dHJpYnV0ZWQtc3RyaW5nLmh0bWwgdGVzdCBzb21ldGltZXMgdGhyb3dzIGV4 Y2VwdGlvbnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTg1MDgxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg QWRkIGluIG1vcmUgcmFuZ2UgY2hlY2tpbmcgaW4gY2FzZSB3ZSBnZXQgYmFjayByYW5nZXMgZnJv bSBzcGVsbCBjaGVja2luZyB0aGF0IGFyZSB3cm9uZy4KKworICAgICAgICAqIGFjY2Vzc2liaWxp dHkvbWFjL1dlYkFjY2Vzc2liaWxpdHlPYmplY3RXcmFwcGVyLm1tOgorICAgICAgICAoQVhBdHRy aWJ1dGVTdHJpbmdTZXRGb250KToKKyAgICAgICAgKEFYQXR0cmlidXRlU3RyaW5nU2V0Q29sb3Ip OgorICAgICAgICAoQVhBdHRyaWJ1dGVTdHJpbmdTZXROdW1iZXIpOgorICAgICAgICAoQVhBdHRy aWJ1dGVTdHJpbmdTZXRCbG9ja3F1b3RlTGV2ZWwpOgorICAgICAgICAoQVhBdHRyaWJ1dGVTdHJp bmdTZXRIZWFkaW5nTGV2ZWwpOgorICAgICAgICAoQVhBdHRyaWJ1dGVTdHJpbmdTZXRFbGVtZW50 KToKKwogMjAxMi0wNC0yNyAgQWxleGFuZGVyIFBhdmxvdiAgPGFwYXZsb3ZAY2hyb21pdW0ub3Jn PgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IEltcGxlbWVudCB0aGUgIkRpc2FibGUgSmF2YVNj cmlwdCIgb3B0aW9uIGluIHRoZSBzZXR0aW5ncyBkaWFsb2cKSW5kZXg6IFNvdXJjZS9XZWJDb3Jl L2FjY2Vzc2liaWxpdHkvbWFjL1dlYkFjY2Vzc2liaWxpdHlPYmplY3RXcmFwcGVyLm1tCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvbWFjL1dlYkFjY2Vzc2liaWxp dHlPYmplY3RXcmFwcGVyLm1tCShyZXZpc2lvbiAxMTU0NDgpCisrKyBTb3VyY2UvV2ViQ29yZS9h Y2Nlc3NpYmlsaXR5L21hYy9XZWJBY2Nlc3NpYmlsaXR5T2JqZWN0V3JhcHBlci5tbQkod29ya2lu ZyBjb3B5KQpAQCAtNTcxLDEwICs1NzEsMTEgQEAKIAogc3RhdGljIHZvaWQgQVhBdHRyaWJ1dGVT dHJpbmdTZXRGb250KE5TTXV0YWJsZUF0dHJpYnV0ZWRTdHJpbmcqIGF0dHJTdHJpbmcsIE5TU3Ry aW5nKiBhdHRyaWJ1dGUsIE5TRm9udCogZm9udCwgTlNSYW5nZSByYW5nZSkKIHsKLSAgICBOU0Rp Y3Rpb25hcnkqIGRpY3Q7Ci0gICAgCisgICAgaWYgKE5TTWF4UmFuZ2UocmFuZ2UpID4gW2F0dHJT dHJpbmcgbGVuZ3RoXSkKKyAgICAgICAgcmV0dXJuOworCiAgICAgaWYgKGZvbnQpIHsKLSAgICAg ICAgZGljdCA9IFtOU0RpY3Rpb25hcnkgZGljdGlvbmFyeVdpdGhPYmplY3RzQW5kS2V5czoKKyAg ICAgICAgTlNEaWN0aW9uYXJ5KiBkaWN0ID0gW05TRGljdGlvbmFyeSBkaWN0aW9uYXJ5V2l0aE9i amVjdHNBbmRLZXlzOgogICAgICAgICAgICAgW2ZvbnQgZm9udE5hbWVdICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAsIE5TQWNjZXNzaWJpbGl0eUZvbnROYW1lS2V5LAogICAgICAgICAgICAg W2ZvbnQgZmFtaWx5TmFtZV0gICAgICAgICAgICAgICAgICAgICAgICAgICAsIE5TQWNjZXNzaWJp bGl0eUZvbnRGYW1pbHlLZXksCiAgICAgICAgICAgICBbZm9udCBkaXNwbGF5TmFtZV0gICAgICAg ICAgICAgICAgICAgICAgICAgICwgTlNBY2Nlc3NpYmlsaXR5VmlzaWJsZU5hbWVLZXksCkBAIC02 MTIsNiArNjEzLDkgQEAKIAogc3RhdGljIHZvaWQgQVhBdHRyaWJ1dGVTdHJpbmdTZXRDb2xvcihO U011dGFibGVBdHRyaWJ1dGVkU3RyaW5nKiBhdHRyU3RyaW5nLCBOU1N0cmluZyogYXR0cmlidXRl LCBOU0NvbG9yKiBjb2xvciwgTlNSYW5nZSByYW5nZSkKIHsKKyAgICBpZiAoTlNNYXhSYW5nZShy YW5nZSkgPiBbYXR0clN0cmluZyBsZW5ndGhdKQorICAgICAgICByZXR1cm47CisKICAgICBpZiAo Y29sb3IpIHsKICAgICAgICAgQ0dDb2xvclJlZiBleGlzdGluZ0NvbG9yID0gKENHQ29sb3JSZWYp IFthdHRyU3RyaW5nIGF0dHJpYnV0ZTphdHRyaWJ1dGUgYXRJbmRleDpyYW5nZS5sb2NhdGlvbiBl ZmZlY3RpdmVSYW5nZTpuaWxdOwogICAgICAgICBDR0NvbG9yUmVmIGNnQ29sb3IgPSBDcmVhdGVD R0NvbG9ySWZEaWZmZXJlbnQoY29sb3IsIGV4aXN0aW5nQ29sb3IpOwpAQCAtNjI1LDYgKzYyOSw5 IEBACiAKIHN0YXRpYyB2b2lkIEFYQXR0cmlidXRlU3RyaW5nU2V0TnVtYmVyKE5TTXV0YWJsZUF0 dHJpYnV0ZWRTdHJpbmcqIGF0dHJTdHJpbmcsIE5TU3RyaW5nKiBhdHRyaWJ1dGUsIE5TTnVtYmVy KiBudW1iZXIsIE5TUmFuZ2UgcmFuZ2UpCiB7CisgICAgaWYgKE5TTWF4UmFuZ2UocmFuZ2UpID4g W2F0dHJTdHJpbmcgbGVuZ3RoXSkKKyAgICAgICAgcmV0dXJuOworCiAgICAgaWYgKG51bWJlcikK ICAgICAgICAgW2F0dHJTdHJpbmcgYWRkQXR0cmlidXRlOmF0dHJpYnV0ZSB2YWx1ZTpudW1iZXIg cmFuZ2U6cmFuZ2VdOwogICAgIGVsc2UKQEAgLTY5NSw2ICs3MDIsOSBAQAogCiBzdGF0aWMgdm9p ZCBBWEF0dHJpYnV0ZVN0cmluZ1NldEJsb2NrcXVvdGVMZXZlbChOU011dGFibGVBdHRyaWJ1dGVk U3RyaW5nKiBhdHRyU3RyaW5nLCBSZW5kZXJPYmplY3QqIHJlbmRlcmVyLCBOU1JhbmdlIHJhbmdl KQogeworICAgIGlmIChOU01heFJhbmdlKHJhbmdlKSA+IFthdHRyU3RyaW5nIGxlbmd0aF0pCisg ICAgICAgIHJldHVybjsKKwogICAgIEFjY2Vzc2liaWxpdHlPYmplY3QqIG9iaiA9IHJlbmRlcmVy LT5kb2N1bWVudCgpLT5heE9iamVjdENhY2hlKCktPmdldE9yQ3JlYXRlKHJlbmRlcmVyKTsKICAg ICBpbnQgcXVvdGVMZXZlbCA9IG9iai0+YmxvY2txdW90ZUxldmVsKCk7CiAgICAgCkBAIC03NDYs NiArNzU2LDkgQEAKICAgICBpZiAoIXJlbmRlcmVyKQogICAgICAgICByZXR1cm47CiAgICAgCisg ICAgaWYgKE5TTWF4UmFuZ2UocmFuZ2UpID4gW2F0dHJTdHJpbmcgbGVuZ3RoXSkKKyAgICAgICAg cmV0dXJuOworICAgIAogICAgIC8vIFNvbWV0aW1lcyB0aGVyZSBhcmUgb2JqZWN0cyBiZXR3ZWVu IHRoZSB0ZXh0IGFuZCB0aGUgaGVhZGluZy4gCiAgICAgLy8gSW4gdGhvc2UgY2FzZXMgdGhlIHBh cmVudCBoaWVyYXJjaHkgc2hvdWxkIGJlIHF1ZXJpZWQgdG8gc2VlIGlmIHRoZXJlIGlzIGEgaGVh ZGluZyBsZXZlbC4KICAgICBpbnQgcGFyZW50SGVhZGluZ0xldmVsID0gMDsKQEAgLTc2NCw2ICs3 NzcsOSBAQAogCiBzdGF0aWMgdm9pZCBBWEF0dHJpYnV0ZVN0cmluZ1NldEVsZW1lbnQoTlNNdXRh YmxlQXR0cmlidXRlZFN0cmluZyogYXR0clN0cmluZywgTlNTdHJpbmcqIGF0dHJpYnV0ZSwgQWNj ZXNzaWJpbGl0eU9iamVjdCogb2JqZWN0LCBOU1JhbmdlIHJhbmdlKQogeworICAgIGlmIChOU01h eFJhbmdlKHJhbmdlKSA+IFthdHRyU3RyaW5nIGxlbmd0aF0pCisgICAgICAgIHJldHVybjsKKwog ICAgIGlmIChvYmplY3QgJiYgb2JqZWN0LT5pc0FjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3QoKSkg ewogICAgICAgICAvLyBtYWtlIGEgc2VyaWFsaXphYmxlIEFYIG9iamVjdAogICAgICAgICAK