Bug 108962

Summary: Add null check to editable in moveCaretSelectionTowardsWindowPoint
Product: WebKit Reporter: David Trainor <dtrainor>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cjhopman, eric, esprehn, lforschler, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

David Trainor
Reported 2013-02-05 11:21:29 PST
Add null check to editable in moveCaretSelectionTowardsWindowPoint
Attachments
Patch (1.56 KB, patch)
2013-02-05 11:22 PST, David Trainor 		no flags
DetailsFormatted DiffDiff
Patch (2.37 KB, patch)
2013-02-06 17:26 PST, David Trainor 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Trainor
Comment 1 2013-02-05 11:22:20 PST
Created attachment 186665 [details] Patch
Build Bot
Comment 2 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
Build Bot
Comment 3 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
Eric Seidel (no email)
Comment 4 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
Eric Seidel (no email)
Comment 5 2013-02-06 16:11:17 PST
Comment on attachment 186665 [details] Patch Can we test this?
David Trainor
Comment 6 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.
David Trainor
Comment 7 2013-02-06 17:26:47 PST
Created attachment 186955 [details] Patch
Elliott Sprehn
Comment 8 2013-02-06 17:36:03 PST
Were there cases where we'd actually do this right now?
David Trainor
Comment 9 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.
Elliott Sprehn
Comment 10 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.
Eric Seidel (no email)
Comment 11 2013-02-06 18:13:00 PST
Comment on attachment 186955 [details] Patch LGTM.
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2013-02-07 04:19:52 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.