Bug 26214 - RenderTextControl: Remove ASSERT for checking that visiblePositionForIndex()'s return is not null.
Summary: RenderTextControl: Remove ASSERT for checking that visiblePositionForIndex()'...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-05 02:53 PDT by Takeshi Yoshino
Modified: 2009-06-10 18:59 PDT (History)
3 users (show)

See Also:


Attachments
Proposed fix for 26214 (4.77 KB, patch)
2009-06-05 03:00 PDT, Takeshi Yoshino
no flags Details | Formatted Diff | Diff
Proposed fix for 26214 (rev2) (4.63 KB, patch)
2009-06-08 20:25 PDT, Takeshi Yoshino
justin.garcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takeshi Yoshino 2009-06-05 02:53:36 PDT
visiblePositionForIndex can return an instance that returns true for
isNotNull(). One of common case is when the corresponding input element has
"-webkit-user-select: none" style attribute. We should allow the case instead
of putting ASSERT.
Comment 1 Takeshi Yoshino 2009-06-05 03:00:03 PDT
Created attachment 31000 [details]
Proposed fix for 26214
Comment 2 Eric Seidel (no email) 2009-06-08 17:57:56 PDT
Comment on attachment 31000 [details]
Proposed fix for 26214

Why does this need to be guarded in an if?  Doesn't VisibleSelection(foo, null) result in an empty selection?  or does that ASSERT too?
Comment 3 Takeshi Yoshino 2009-06-08 20:24:38 PDT
(In reply to comment #2)
> (From update of attachment 31000 [details] [review])
> Why does this need to be guarded in an if?  Doesn't VisibleSelection(foo, null)
> result in an empty selection?  or does that ASSERT too?
> 

Yes, creation of VisibleSelection can be put outside if-clause. Fixed.

The line
ASSERT(startPosition.deepEquivalent().node()->shadowAncestorNode() == node() && endPosition.deepEquivalent().node()->shadowAncestorNode() == node());
must be guarded into if-clause because deepEquivalent() can be null. In fact, I observe shadowAncestorNode() called with this=null.
Comment 4 Takeshi Yoshino 2009-06-08 20:25:28 PDT
Created attachment 31079 [details]
Proposed fix for 26214 (rev2)
Comment 5 Justin Garcia 2009-06-09 18:09:44 PDT
Comment on attachment 31079 [details]
Proposed fix for 26214 (rev2)

r=me
Comment 6 Takeshi Yoshino 2009-06-10 02:17:54 PDT
Thank you. Could you land this for me?
Comment 7 Brent Fulgham 2009-06-10 11:21:34 PDT
Landed in @r44578.
Comment 8 Takeshi Yoshino 2009-06-10 18:59:38 PDT
(In reply to comment #7)
> Landed in @r44578.
> 

Thank you.