135281 2014-07-24 19:58:28 -0700 Crashes under scanSelectionForTelephoneNumbers in Range::text() on some sites 2014-07-24 20:13:00 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton beidson enrica rniwa oldest_to_newest 1024754 0 thorton 2014-07-24 19:58:28 -0700 Steps to Reproduce: 1. Load http://dict.leo.org/#/search=apple&searchLoc=0&resultOrder=basic&multiwordShowSingle=on <rdar://problem/17803347> 1024758 1 235493 thorton 2014-07-24 20:01:48 -0700 Created attachment 235493 patch 1024760 2 235493 rniwa 2014-07-24 20:03:59 -0700 Comment on attachment 235493 patch View in context: https://bugs.webkit.org/attachment.cgi?id=235493&action=review > Source/WebCore/editing/Editor.cpp:3396 > RefPtr<Range> extendedRange = extendedSelection.toNormalizedRange(); > > + if (!extendedRange) { Looks like we can move extendedRange into the if expression and then wrap the following ~8 lines of code inside the if statement instead. That way, we make exactly one call to selectedTelephoneNumberRangesChanged. 1024767 3 235493 thorton 2014-07-24 20:11:43 -0700 Comment on attachment 235493 patch View in context: https://bugs.webkit.org/attachment.cgi?id=235493&action=review >> Source/WebCore/editing/Editor.cpp:3396 >> + if (!extendedRange) { > > Looks like we can move extendedRange into the if expression and then wrap the following ~8 lines of code inside the if statement instead. > That way, we make exactly one call to selectedTelephoneNumberRangesChanged. I'm teetering back and forth between the two options, because I prefer the early return but at the same time having two calls to selectedTelephoneNumberRangesChanged is kind of annoying. That said, there's another early return that calls selectedTelephoneNumberRangesChanged above, and I would never make this change to that one, so I think I'm going to keep it consistent. Thanks for your help and review! 1024768 4 thorton 2014-07-24 20:13:00 -0700 http://trac.webkit.org/changeset/171577 235493 2014-07-24 20:01:48 -0700 2014-07-24 20:11:43 -0700 patch range.diff text/plain 1512 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBmNmM4NzJiLi40NDFlMjhmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTQtMDctMjQgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgQ3Jhc2hlcyB1bmRlciBzY2FuU2VsZWN0aW9uRm9yVGVsZXBob25lTnVtYmVycyBp biBSYW5nZTo6dGV4dCgpIG9uIHNvbWUgc2l0ZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNTI4MQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTc4 MDMzNDc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBlZGl0aW5nL0VkaXRvci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpFZGl0b3I6OnNjYW5TZWxl Y3Rpb25Gb3JUZWxlcGhvbmVOdW1iZXJzKToKKyAgICAgICAgdG9Ob3JtYWxpemVkUmFuZ2UgaXMg bm90IGd1YXJhbnRlZWQgdG8gcmV0dXJuIGEgbm9uLW51bGwgcmFuZ2UuCisgICAgICAgIElmIGl0 IHJldHVybnMgbnVsbCwgcGFzcyB0aGUgZW1wdHkgbWFya2VkUmFuZ2VzIGRvd24gdG8gdGhlIGNs aWVudCBhcyBvdXIgbmV3IHNldC4KKwogMjAxNC0wNy0yNCAgV2Vuc29uIEhzaWVoICA8d2Vuc29u X2hzaWVoQGFwcGxlLmNvbT4KIAogICAgICAgICBMZXQgV2hlZWxFdmVudCB3cmFwIGEgUGxhdGZv cm1XaGVlbEV2ZW50CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9lZGl0aW5nL0VkaXRvci5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9lZGl0aW5nL0VkaXRvci5jcHAKaW5kZXggZmVhODliOC4uMGU5 OTZjMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvZWRpdGluZy9FZGl0b3IuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL2VkaXRpbmcvRWRpdG9yLmNwcApAQCAtMzM5Myw2ICszMzkzLDExIEBA IHZvaWQgRWRpdG9yOjpzY2FuU2VsZWN0aW9uRm9yVGVsZXBob25lTnVtYmVycygpCiAgICAgZXh0 ZW5kZWRTZWxlY3Rpb24uc2V0RW5kKGVuZCk7CiAgICAgUmVmUHRyPFJhbmdlPiBleHRlbmRlZFJh bmdlID0gZXh0ZW5kZWRTZWxlY3Rpb24udG9Ob3JtYWxpemVkUmFuZ2UoKTsKIAorICAgIGlmICgh ZXh0ZW5kZWRSYW5nZSkgeworICAgICAgICBjbGllbnQoKS0+c2VsZWN0ZWRUZWxlcGhvbmVOdW1i ZXJSYW5nZXNDaGFuZ2VkKG1hcmtlZFJhbmdlcyk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisK ICAgICBzY2FuUmFuZ2VGb3JUZWxlcGhvbmVOdW1iZXJzKCpleHRlbmRlZFJhbmdlLCBleHRlbmRl ZFJhbmdlLT50ZXh0KCksIG1hcmtlZFJhbmdlcyk7CiAKICAgICAvLyBPbmx5IGNvbnNpZGVyIHJh bmdlcyB3aXRoIGEgZGV0ZWN0ZWQgdGVsZXBob25lIG51bWJlciBpZiB0aGV5IG92ZXJsYXAgd2l0 aCB0aGUgYWN0dWFsIHNlbGVjdGlvbiByYW5nZS4K