There are a few pieces of code that don't expect documentElement() to return NULL, and these will lead to calling methods on a null pointer. I've found 4 cases, but there are probably more.
Confirmed with r34486.
Created attachment 21683 [details]
Patch fixes the 4 issues in a pretty straightforward way. I'm not sure about throwing HIERARCHY_REQUEST_ERR in setBody, maybe there is something better to throw?
I can make a layout test if needed.
Comment on attachment 21683 [details]
This needs a ChangeLog, see:
And a test case. (Also, next time, mark the change as r=? if you'd like it to be seen in the review queue. :)
Created attachment 21687 [details]
Comment on attachment 21687 [details]
Looks fine. There are various small problems such as:
+ * LayoutTests\fast\dom\documentElement-null-expected.txt: Added.
Slashes are backwards here. Paths are from the top level, but should be relative to the ChangeLog file. LayoutTests needs its own separate ChangeLog file, not entries in the WebCore ChangeLog file.
+ if (const Element *doc = static_cast<const Document *>(this)->documentElement())
+ return doc->lookupPrefix(namespaceURI);
+ return String();
We put the * next to the type name, although the old code didn't do that. The newly added braces aren't needed -- the if statement already scopes the variable declared inside it.
I'd also suggest changing documentElement-null.html to create a document and put a message in at the end indicating success.
But I think the patch is good enough as-is that I'm going to review+ it. The person landing it can probably fix a few things, or someone could do a new patch.
Created attachment 21691 [details]
Comment on attachment 21691 [details]
+ if (window.LayoutTestController)
The letter "L" has to be lowercase here, or it won't work.
Why LF style? We don't use that style for other files.
+ if (const Element* doc = static_cast<const Document*>(this)->documentElement())
+ return doc->isDefaultNamespace(namespaceURI);
We normally use "doc" for "document". Using "doc" for "documentElement" is unnecessarily confusing. Maybe "de" or "documentElement" or even just "e" or "element". Also really no need for const in the type of the local variable.
review- because of the letter "L" problem. I have no idea why the test worked!
Do you really want to go without a name in the change log? I'd prefer "Anonymous" to "danceoff" for the name here.
Created attachment 21729 [details]
Patch attempt number 300
Comment on attachment 21729 [details]
Patch attempt number 300
What happens now, will it get landed?
This was landed in <http://trac.webkit.org/projects/webkit/changeset/34789>.
*** Bug 20853 has been marked as a duplicate of this bug. ***