WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
135220
REGRESSION (
r171376
): Sometimes we detect less than the whole phone number
https://bugs.webkit.org/show_bug.cgi?id=135220
Summary
REGRESSION (r171376): Sometimes we detect less than the whole phone number
Tim Horton
Reported
2014-07-23 16:50:07 PDT
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
>
Attachments
patch
(5.88 KB, patch)
2014-07-23 16:54 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
patch
(5.84 KB, patch)
2014-07-23 17:08 PDT
,
Tim Horton
beidson
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Tim Horton
Comment 1
2014-07-23 16:54:48 PDT
Created
attachment 235391
[details]
patch
Tim Horton
Comment 2
2014-07-23 17:08:43 PDT
Created
attachment 235393
[details]
patch
Brady Eidson
Comment 3
2014-07-23 17:12:42 PDT
This sounds great, and is probably what we should've done in the first place.
Brady Eidson
Comment 4
2014-07-23 17:14:08 PDT
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
Comment 5
2014-07-23 17:22:22 PDT
(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
Comment 6
2014-07-23 17:24:47 PDT
(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
Comment 7
2014-07-23 17:25:13 PDT
http://trac.webkit.org/changeset/171499
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug