| Summary: | Add null check to editable in moveCaretSelectionTowardsWindowPoint | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Trainor <dtrainor> | ||||||
| Component: | New Bugs | Assignee: | 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
David Trainor
2013-02-05 11:21:29 PST
Created attachment 186665 [details]
Patch
Comment on attachment 186665 [details] Patch Attachment 186665 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16366916 Comment on attachment 186665 [details] Patch Attachment 186665 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16368837 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 on attachment 186665 [details]
Patch
Can we test this?
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. Created attachment 186955 [details]
Patch
Were there cases where we'd actually do this right now? 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. (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 on attachment 186955 [details]
Patch
LGTM.
Comment on attachment 186955 [details] Patch Clearing flags on attachment: 186955 Committed r142093: <http://trac.webkit.org/changeset/142093> All reviewed patches have been landed. Closing bug. |