Bug 26214 – RenderTextControl: Remove ASSERT for checking that visiblePositionForIndex()'s return is not null.

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Takeshi Yoshino]

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.

[Takeshi Yoshino]

Created attachment 31000 [details]
Proposed fix for 26214

[Eric Seidel (no email)]

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?

[Takeshi Yoshino]

(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.

[Takeshi Yoshino]

Created attachment 31079 [details]
Proposed fix for 26214 (rev2)

[Justin Garcia]

Comment on attachment 31079 [details]
Proposed fix for 26214 (rev2)

r=me

[Takeshi Yoshino]

Thank you. Could you land this for me?

[Brent Fulgham]

Landed in @r44578.

[Takeshi Yoshino]

(In reply to comment #7)
> Landed in @r44578.
> 

Thank you.