Bug 135220 – REGRESSION (r171376): Sometimes we detect less than the whole phone number

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Tim Horton]

This is happening because the detection code will happily say that (###) ###-### is a real phone number (it probably is, somewhere), and our approach of using TextIterator and adjusting the ranges sometimes results in a fragment of a number being matched against first — before the whole number is passed in. The code in scanSelectionForTelephoneNumbers doesn't really know how to merge these, and we have no way to predict what the detection code will do, so we should instead hand *the whole extended selection* in one chunk to the detection code, so it can eagerly detect all of the phone numbers.

Doing this made it clear that the code to extend the selection was quite broken in the case of a directional selection (if it was a selection from right to left, the extension code would actually squish the selection instead of expanding it).

I haven't found any cases that this makes *worse*, and it is significantly simpler.

<rdar://problem/17783423>

[Tim Horton]

Created attachment 235391 [details]
patch

[Tim Horton]

Created attachment 235393 [details]
patch

[Brady Eidson]

This sounds great, and is probably what we should've done in the first place.

[Brady Eidson]

Comment on attachment 235393 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235393&action=review

> Source/WebCore/editing/Editor.cpp:3438
> +

Really necessary?  Paragraphing seemed okay before.

[Tim Horton]

(In reply to comment #4)
> (From update of attachment 235393 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235393&action=review
> 
> > Source/WebCore/editing/Editor.cpp:3438
> > +
> 
> Really necessary?  Paragraphing seemed okay before.

Not necessary! Just left over from some debugging logging that I failed to remove entirely.

Thanks for your review!

[Tim Horton]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 235393 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=235393&action=review
> > 
> > > Source/WebCore/editing/Editor.cpp:3438
> > > +
> > 
> > Really necessary?  Paragraphing seemed okay before.
> 
> Not necessary! Just left over from some debugging logging that I failed to remove entirely.

Though somehow I managed to land it anyway :|

> Thanks for your review!

[Tim Horton]

http://trac.webkit.org/changeset/171499