Bug 98907

Summary: Document calls createElement with the wrong parameters.
Product: WebKit Reporter: Mike West <mkwst>
Component: New BugsAssignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, jochen, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 98050    
Attachments:
Description Flags
Patch none

Mike West
Reported 2012-10-10 08:01:49 PDT
Document calls createElement with the wrong parameters.
Attachments
Patch (1.70 KB, patch)
2012-10-10 08:06 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 1 2012-10-10 08:06:25 PDT
Created attachment 167998 [details] Patch
Mike West
Comment 2 2012-10-10 08:10:58 PDT
This isn't so trivial as the others I've seen today. Document::importNode calls Document::createElement with a QualifiedName and ExceptionCode. The Document::createElement that takes a QualifiedName doesn't generate an exception; the second argument is a bool, which the ExceptionCode autocasts into. There is, however, a Document::createElement that takes an AtomicString and an Exception code. I wonder if that was the original intent. CCing Kent Tamura, who looks like he wrote that code in the first place. Hi Kent! Would you mind taking a look at this?
jochen
Comment 3 2012-10-10 08:21:23 PDT
(In reply to comment #2) > This isn't so trivial as the others I've seen today. > > Document::importNode calls Document::createElement with a QualifiedName and ExceptionCode. The Document::createElement that takes a QualifiedName doesn't generate an exception; the second argument is a bool, which the ExceptionCode autocasts into. > > There is, however, a Document::createElement that takes an AtomicString and an Exception code. I wonder if that was the original intent. CCing Kent Tamura, who looks like he wrote that code in the first place. > > Hi Kent! Would you mind taking a look at this? I could imagine that you'd need to invoke the version that actually takes a ec: if the document is an xml doc, a different type of element needs to be created.
Mike West
Comment 4 2012-10-10 08:27:21 PDT
(In reply to comment #3) > (In reply to comment #2) > > This isn't so trivial as the others I've seen today. > > > > Document::importNode calls Document::createElement with a QualifiedName and ExceptionCode. The Document::createElement that takes a QualifiedName doesn't generate an exception; the second argument is a bool, which the ExceptionCode autocasts into. > > > > There is, however, a Document::createElement that takes an AtomicString and an Exception code. I wonder if that was the original intent. CCing Kent Tamura, who looks like he wrote that code in the first place. > > > > Hi Kent! Would you mind taking a look at this? > > I could imagine that you'd need to invoke the version that actually takes a ec: if the document is an xml doc, a different type of element needs to be created. So we likely need 'oldElement->localName()' instead of 'oldElement->tagQName()'?
Kent Tamura
Comment 5 2012-10-10 17:55:32 PDT
The purpose of http://trac.webkit.org/changeset/104275 was to use createElement with QualifiedName argument. So the patch is correct.
Kent Tamura
Comment 6 2012-10-10 17:55:45 PDT
Comment on attachment 167998 [details] Patch ok
Mike West
Comment 7 2012-10-10 21:38:26 PDT
Comment on attachment 167998 [details] Patch Alright, thanks for taking a look! Throwing this into the queue.
WebKit Review Bot
Comment 8 2012-10-10 21:56:52 PDT
Comment on attachment 167998 [details] Patch Clearing flags on attachment: 167998 Committed r131015: <http://trac.webkit.org/changeset/131015>
WebKit Review Bot
Comment 9 2012-10-10 21:56:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.