Bug 13534 - REGRESSION: setting innerHTML to null doesn't work correctly
Summary: REGRESSION: setting innerHTML to null doesn't work correctly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-04-28 08:12 PDT by Alexey Proskuryakov
Modified: 2007-05-16 10:05 PDT (History)
1 user (show)

See Also:

Attachments
test case (136 bytes, text/html)
2007-04-28 08:12 PDT, Alexey Proskuryakov 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-04-28 08:12:00 PDT 
Setting innerHTML to null sets it to "null" instead.

As discussed in bug 13448, this is likely to be a problem with all settable properties, but I didn't see any follow-up bug filed (or did I just miss it?).
Comment 1 Alexey Proskuryakov 2007-04-28 08:12:31 PDT 
Created attachment 14241 [details]
test case
Comment 2 Darin Adler 2007-04-28 10:52:46 PDT 
The binding for innerHTML is not automatically generated. So unlike bug 13448, [ConvertNullToNullString] in the IDL file is not the way to fix the bug (although it would not hurt to have that there to prepare for the day when we do auto-generate the binding).

The fix for this bug goes in JSHTMLElement::putValueProperty in kjs_html.cpp. Instead of value->toString(), that function needs to use valueToStringWithNullCheck() in at least some of the cases.

This is a general problem area for our bindings for functions and properties that take and return strings. Many are untested about whether they should accept null or undefined as a special values or return null or undefined rather than empty string in some cases.

Another problem area is that is untested is whether functions return null or undefined. I believe we return undefined in many cases where null would be better/correct.
Comment 3 Darin Adler 2007-04-28 10:56:01 PDT 
<rdar://problem/5168940>
Comment 4 Sam Weinig 2007-04-29 08:04:25 PDT 
A more general follow up bug to bug 13448 has been filed as bug 13541.
Comment 5 Sam Weinig 2007-05-16 10:05:16 PDT 
Fixed by the first patch in bug 13722, landed in r21489.