[webkit-reviews] review denied: [Bug 106361] [Mac] svg/custom/text-use-click-crash.xhtml added by r139029 hits assertion in enclosingTextFormControl : [Attachment 181879] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Hajime Morrita <morrita at google.com>'s
request for review:
Bug 106361: [Mac] svg/custom/text-use-click-crash.xhtml added by r139029 hits
assertion in enclosingTextFormControl
https://bugs.webkit.org/show_bug.cgi?id=106361

Attachment 181879: Patch
https://bugs.webkit.org/attachment.cgi?id=181879&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=181879&action=review


> Source/WebCore/ChangeLog:9
> +	   In most case this assuption makes sense but it can be broken when
isInPasswordField().

Typo: assuption

> Source/WebCore/ChangeLog:10
> +	   This change checks precondiiton before invoking
enclosingTextFormControl() there.

Typo: precondiiton

> Source/WebCore/editing/FrameSelection.cpp:1676
> +    if (!start().containerNode())
> +	   return false;
>      HTMLTextFormControlElement* textControl =
enclosingTextFormControl(start());

I think we should rethink and loosen the assertion in enclosingTextFormControl
instead of doing this. The actual implementation of that function is perfectly
fine, it’s just got an overzealous assertion in it.

I hate the idea of checking something here that’s also checked first thing in
the enclosingTextFormControl function, just because the assertion in there is
too strong.

Since Ryosuke wrote that assertion, I’d like him to comment on this.