Bug 108962 - Add null check to editable in moveCaretSelectionTowardsWindowPoint
Summary: Add null check to editable in moveCaretSelectionTowardsWindowPoint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-05 11:21 PST by David Trainor
Modified: 2013-02-07 04:19 PST (History)
5 users (show)

See Also:

Attachments
Patch (1.56 KB, patch)
2013-02-05 11:22 PST, David Trainor 		no flags Details | Formatted Diff | Diff
Patch (2.37 KB, patch)
2013-02-06 17:26 PST, David Trainor 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Trainor 2013-02-05 11:21:29 PST 
Add null check to editable in moveCaretSelectionTowardsWindowPoint
Comment 1 David Trainor 2013-02-05 11:22:20 PST 
Created attachment 186665 [details]
Patch
Comment 2 Build Bot 2013-02-05 12:37:14 PST 
Comment on attachment 186665 [details]
Patch

Attachment 186665 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16366916
Comment 3 Build Bot 2013-02-05 13:43:39 PST 
Comment on attachment 186665 [details]
Patch

Attachment 186665 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16368837
Comment 4 Eric Seidel (no email) 2013-02-06 16:11:05 PST 
The Win-EWS failure seems unrelated:
1>..\cg\ImageDiffCG.cpp(37) : fatal error C1083: Cannot open include file: 'wtf/Platform.h': No such file or directory
Comment 5 Eric Seidel (no email) 2013-02-06 16:11:17 PST 
Comment on attachment 186665 [details]
Patch

Can we test this?
Comment 6 David Trainor 2013-02-06 16:15:30 PST 
Good point.  I can put in a simple check that tries calling this when there's no selection and just makes sure it doesn't crash.
Comment 7 David Trainor 2013-02-06 17:26:47 PST 
Created attachment 186955 [details]
Patch
Comment 8 Elliott Sprehn 2013-02-06 17:36:03 PST 
Were there cases where we'd actually do this right now?
Comment 9 David Trainor 2013-02-06 17:57:57 PST 
We're seeing this happen because we're trying to clear the selection state then handling a different touch event that moves the caret.  We're not getting the actual state change back from webkit fast enough to update the front end before we get the touch event which tries to move the caret.  That needs to be fixed there as well but I don't think we should crash here either way.
Comment 10 Elliott Sprehn 2013-02-06 18:03:22 PST 
(In reply to comment #9)
> We're seeing this happen because we're trying to clear the selection state then handling a different touch event that moves the caret.  We're not getting the actual state change back from webkit fast enough to update the front end before we get the touch event which tries to move the caret.  That needs to be fixed there as well but I don't think we should crash here either way.

Seems reasonable.
Comment 11 Eric Seidel (no email) 2013-02-06 18:13:00 PST 
Comment on attachment 186955 [details]
Patch

LGTM.
Comment 12 WebKit Review Bot 2013-02-07 04:19:48 PST 
Comment on attachment 186955 [details]
Patch

Clearing flags on attachment: 186955

Committed r142093: <http://trac.webkit.org/changeset/142093>
Comment 13 WebKit Review Bot 2013-02-07 04:19:52 PST 
All reviewed patches have been landed.  Closing bug.