Bug 9358 – REGRESSION: Assertion failure in HTMLInputElement::setValueFromRenderer (value == constrainValue(value)) when deleting all text

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

I hit this assert when I try to delete the X in the field using the delete key:

ASSERTION FAILED: value == constrainValue(value) (WebCore/html/HTMLInputElement.cpp:1068 void WebCore::HTMLInputElement::setValueFromRenderer(const WebCore::String&))

The root cause seems to be that now when you delete the last character, a <br> placeholder is inserted.

[Justin Garcia]

Look like textContent returns "\n" for a br even if its collapsed.  value = "\n" and constrainValue(value) = "".

[David Kilzer (:ddkilzer)]

Grrr!  I Just ran into this on Bugzilla's simple search page, and it killed my browser!

[David Kilzer (:ddkilzer)]

This is also a regression (so to speak).

[David Kilzer (:ddkilzer)]

*** Bug 9429 has been marked as a duplicate of this bug. ***

*** Bug 9542 has been marked as a duplicate of this bug. ***

[David Kilzer (:ddkilzer)]

Created attachment 9089 [details]
Patch v1 (no changelog; no test)

I crash from this assertion daily on my debug build.  Tell me what's wrong with this patch so it can be fixed properly.  :)

[Darin Adler]

Comment on attachment 9089 [details]
Patch v1 (no changelog; no test)

It's driving me crazy too!

This is an acceptable workaround for the problem, but not the right way to fix the problem.

I don't want an extra call to constrainValue here in the production code in the long term.

I'd prefer isEmpty() to length() == 0, and I'd like a comment here that explains exactly why we need a special case.

Perhaps a better way to do it would be to add code like this before the assert:

    // Workaround for bug where trailing \n is included in the result of textContent.
    // <URL of a new bug report> 
    if (value == "\n")
        value = "";

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
>     // Workaround for bug where trailing \n is included in the result of
> textContent.
>     // <URL of a new bug report>

Bug 9661 filed for the follow-up issue.

[David Kilzer (:ddkilzer)]

Created attachment 9106 [details]
Patch v2

Patch v2 includes ChangeLogs, a layout test, and addresses Darin's comments.

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> Patch v2 includes ChangeLogs, a layout test, and addresses Darin's comments.

It also passed all layout tests.

[Darin Adler]

Comment on attachment 9106 [details]
Patch v2

OK. r=me

[David Kilzer (:ddkilzer)]

Committed revision 15111!