Bug 5987

Summary: <applet> is broken in xhtml
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: DOMAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, c.petersen87, darin, julien.palmas
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Proposed patch
none
Better patch (correctly handles implicit elements) darin: review+

Eric Seidel (no email)
Reported 2005-12-07 04:11:31 PST
<applet> is broken in xhtml closeRenderer() is never being called by the xml parser.
Attachments
Proposed patch (640 bytes, patch)
2005-12-07 04:12 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Better patch (correctly handles implicit elements) (946 bytes, patch)
2005-12-07 04:36 PST, Eric Seidel (no email) 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2005-12-07 04:12:35 PST
Created attachment 4985 [details] Proposed patch
Eric Seidel (no email)
Comment 2 2005-12-07 04:19:10 PST
Comment on attachment 4985 [details] Proposed patch This line should actually probably go after the implicit table node escaping code right below it in the current patch. However another question raised by this patch, is why are we supporting implicit table element construction in xhtml? Hyatt or darin should look at this one.
Eric Seidel (no email)
Comment 3 2005-12-07 04:25:58 PST
Comment on attachment 4985 [details] Proposed patch Actually, I think the logic should be re-written to step out of implicit elements, then call closeRenderer(), then step out to your parent... right now I bet <applet><tr /> </applet> will fail. :)
Eric Seidel (no email)
Comment 4 2005-12-07 04:30:00 PST
Comment on attachment 4985 [details] Proposed patch Actually, implicit elements (<tbody>) are only created between <table> and <tr>, so we're safe here. However the fix would still probably be "more correct" to do what I mentioned above.
Eric Seidel (no email)
Comment 5 2005-12-07 04:35:52 PST
Comment on attachment 4985 [details] Proposed patch Posting a better patch.
Eric Seidel (no email)
Comment 6 2005-12-07 04:36:28 PST
Created attachment 4986 [details] Better patch (correctly handles implicit elements)
Eric Seidel (no email)
Comment 7 2005-12-07 04:37:07 PST
Comment on attachment 4986 [details] Better patch (correctly handles implicit elements) Here is a better patch which correctly handles implicit elements. Dave or Darin should take a look at this one.
Darin Adler
Comment 8 2005-12-07 09:58:30 PST
Comment on attachment 4986 [details] Better patch (correctly handles implicit elements) There are three changes here. One is to call closeRenderer. That looks fine. Another is to set m_currentNode to the grandparent of the implicit node rather than the parent of the implicit node. That also seems fine, because we need to close the element. The third change is to remove NULL checks. That one worries me a bit, but I guess we can count on not getting an endElement callback unless we have successfully started and element. It's worth looking closely at that. What happens if the startElement fails? r=me
Eric Seidel (no email)
Comment 9 2005-12-07 14:44:32 PST
I could add an ASSERT that m_currentNode->parentNode() != 0 if you'd like. But from what I understand this code never should get called in an error case. I've also tried with some xml docs missing opening tags, and not hit any crashes.
Eric Seidel (no email)
Comment 10 2005-12-08 01:06:49 PST
*** Bug 5835 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
Comment 11 2005-12-08 18:26:46 PST
*** Bug 3332 has been marked as a duplicate of this bug. ***
Lucas Forschler
Comment 12 2019-02-06 09:03:27 PST
Mass moving XML DOM bugs to the "DOM" Component.
Note You need to log in before you can comment on or make changes to this bug.