Some values used with setAttribute() cause the attribute to be removed (null strings vs. empty strings)

• Component: DOM
• Status: VERIFIED FIXED

[Vicki Murley]

This bug is also in Radar as <rdar://3885811>

Summary:
Some values used with setAttribute() will cause the attribute to be removed. 

In the test case provided, an <input type='text'> tag has an attribute named startval and its value is an 
empty string. The <input> tag does not have a value attribute, but when the value attribute is queried 
indicates that it is an empty string, length of zero and type of string. 

When the <input> value is used as the value parameter for a setAttribute() call on the <input> tag for 
the startval attribute, Safari removes the startval attribute from the <input> tag.

In testing with other <input> attributes, found that size and defaultValue also displayed this behavior 
(when they were not included in the html). When the value attribute was included in the html, the 
startval attribute was not removed.

Steps to reproduce:
Attached is zattribute.htm which duplicates the issue. Install zattribute.htm to a webserver and execute 
via the command line to observe the results.

Expected results:
I expected Safari to not remove the attribute on the <input> tag.

Actual results:
Safari removed the attribute from the <input> tag. 

Workaround:
If the value attribute was placed on the <input> tag, everything would have worked OK.

Isolation:
Fails on Safari. Works OK on Firefox (Macintosh & Windows), Netscape 7.x (Macintosh & Windows) and 
Internet Explorer.

Darin Adler:
This has something to do with empty strings vs. null strings in our DOM implementations.

[Vicki Murley]

Created attachment 3006 [details]
test case

[David Kilzer (:ddkilzer)]

This appears to be working in ToT for Subversion revision r12256.

[David Kilzer (:ddkilzer)]

Created attachment 7027 [details]
Build WebKit r9638 on Mac OS X 10.4.5 with Xcode 2.2.1

In order to write a failing test case, I needed to build an older version of WebKit than what was available on the oldest nightly.  (WebKit-CVS-2005-10-01 03-27-01 GMT.dmg did not exhibit this bug.)

This patch is what was required to build WebKit r9638 on Mac OS X 10.4.5 with Xcode 2.2.1.  (I chose this revision because I thought r9639 might have fixed the problem, although this probably isn't the case after writing another test case.)  Note that this patch includes the build fix from r9641 in WebKit/WebView.subproj/WebFrame.m.

Also note that there are "problems" when running this version of WebKit with Safari 2.0.3.  Clicking links causes 100% CPU usage and closing windows simply doesn't work.  However, it was sufficient to write a failing test case.  I'm including it in case others want to duplicate the work.

[David Kilzer (:ddkilzer)]

Created attachment 7029 [details]
Patch v1 (test case only)

Patch with only test case and changelog entry.  The bug was fixed between r9638 (2005-07-09) and the first nightly build (WebKit-CVS-2005-10-01 03-27-01 GMT.dmg).

[David Kilzer (:ddkilzer)]

Created attachment 7030 [details]
Test results output from r9638

[Maciej Stachowiak]

Comment on attachment 7029 [details]
Patch v1 (test case only)

r=me
good to land this test

[Darin Adler]

I think it's r10084 that fixed this.

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> I think it's r10084 that fixed this.

I updated to r10083 and built WebKit.  This revision still fails as seen in Attachment 7030 [details].

I updated to r10084 (actually r10086 since it had build fixes for r10084), rebuilt, and confirmed that this revision DID fix this bug!

[David Kilzer (:ddkilzer)]

Created attachment 7085 [details]
Patch to fix ChangeLog entry

Fix LayoutTests/ChangeLog entry per information found in Comment 8.  Does this matter?

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> I updated to r10084 (actually r10086 since it had build fixes for r10084),
> rebuilt, and confirmed that this revision DID fix this bug!

Revision r10084 fixed Bug 4313.

[Darin Adler]

Comment on attachment 7085 [details]
Patch to fix ChangeLog entry

No, it doesn't matter, but I guess we can fix this anyway.

[David Kilzer (:ddkilzer)]

Verified fixed (test checked into svn and passes on r13385).