| Summary: | Crashes under scanSelectionForTelephoneNumbers in Range::text() on some sites | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||
| Component: | WebCore Misc. | Assignee: | Tim Horton <thorton> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | beidson, enrica, rniwa | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Tim Horton
2014-07-24 19:58:28 PDT
Created attachment 235493 [details]
patch
Comment on attachment 235493 [details] 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. Comment on attachment 235493 [details] 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! |