Bug 50095 – Assertion failure by resetting <output> twice

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

[Kent Tamura]

<form action="output.html">
<output value=foo></output>
<input type=reset>
</form>

Load the above HTML on debug build WebKit, and click the reset button twice.

ASSERTION FAILED: !m_isSetTextContentInProgress
(..../WebKit/WebCore/html/HTMLOutputElement.cpp:125 void WebCore::HTMLOutputElement::setTextContentInternal(const WTF::String&))

[Kenichi Ishibashi]

Created attachment 74898 [details]
Patch

[Kent Tamura]

Comment on attachment 74898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74898&action=review

> LayoutTests/fast/forms/script-tests/output-reset-assertion-failed.js:10
> +form.reset();
> +

We had better have
 testPassed('Not crashed');
or something.

> WebCore/html/HTMLOutputElement.cpp:91
> +    if (m_isSetTextContentInProgress)
> +        return;

Really?
Does an HTML page have a change to call reset() before clearing m_isSetTextContentInProgress?

[Kenichi Ishibashi]

Comment on attachment 74898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74898&action=review

Thank you for prompt review. I'll revise the patch soon.

>> LayoutTests/fast/forms/script-tests/output-reset-assertion-failed.js:10
>> +
> 
> We had better have
>  testPassed('Not crashed');
> or something.

Done.

>> WebCore/html/HTMLOutputElement.cpp:91
>> +        return;
> 
> Really?
> Does an HTML page have a change to call reset() before clearing m_isSetTextContentInProgress?

I completely misunderstood the cause. The cause is there is no chance to clear the flag when the value and the default value are the same. Thank you for letting me aware of that.

[Kenichi Ishibashi]

Created attachment 74899 [details]
Patch V1

[Kent Tamura]

Probably the same reason,

<output value=foo></output>

var o = document.getElementsByTagName('output')[0]
o.value = "";
o.value = "";

causes an assertion failure.

[Kent Tamura]

(In reply to comment #5)
> Probably the same reason,
> 
> <output value=foo></output>
> 
> var o = document.getElementsByTagName('output')[0]
> o.value = "";
> o.value = "";

o.value = "";
o.value = "foo";

also fails.

[Kenichi Ishibashi]

Created attachment 74900 [details]
Patch V2

[Kenichi Ishibashi]

Kent-san,

Thanks a lot for another cases. I've revised the patch.

(In reply to comment #6)
> (In reply to comment #5)
> > Probably the same reason,
> > 
> > <output value=foo></output>
> > 
> > var o = document.getElementsByTagName('output')[0]
> > o.value = "";
> > o.value = "";
> 
> o.value = "";
> o.value = "foo";
> 
> also fails.

[Kent Tamura]

Comment on attachment 74900 [details]
Patch V2

Looks ok.  Thanks.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 74900 [details]:

fast/profiler/throw-exception-from-eval.html

Please file bugs against the tests.  These tests were authored by [e-mail], [e-mail], and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 74900 [details]
Patch V2

Clearing flags on attachment: 74900

Committed r72750: <http://trac.webkit.org/changeset/72750>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.