RESOLVED FIXED 139561
REGRESSION(r160182): Fragment parser doesn't close a form element with a close tag
https://bugs.webkit.org/show_bug.cgi?id=139561
Summary REGRESSION(r160182): Fragment parser doesn't close a form element with a clos...
Ryosuke Niwa
Reported 2014-12-11 14:39:18 PST
http://trac.webkit.org/r160182 caused a regression in which a form element starts with <form> but doesn't get closed with </form>.
Attachments
Fixes the bug (6.36 KB, patch)
2014-12-11 15:05 PST, Ryosuke Niwa
no flags
Reverted the scheme changes (5.25 KB, patch)
2014-12-11 15:07 PST, Ryosuke Niwa
no flags
Ryosuke Niwa
Comment 1 2014-12-11 14:39:35 PST
Ryosuke Niwa
Comment 2 2014-12-11 14:39:42 PST
Ryosuke Niwa
Comment 3 2014-12-11 15:05:10 PST
Created attachment 243152 [details] Fixes the bug
Ryosuke Niwa
Comment 4 2014-12-11 15:07:19 PST
Created attachment 243153 [details] Reverted the scheme changes
Benjamin Poulain
Comment 5 2014-12-11 16:04:28 PST
*** Bug 137337 has been marked as a duplicate of this bug. ***
Darin Adler
Comment 6 2014-12-12 19:53:28 PST
Comment on attachment 243153 [details] Reverted the scheme changes View in context: https://bugs.webkit.org/attachment.cgi?id=243153&action=review > Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 > + m_form = static_pointer_cast<HTMLFormElement>(element.release()); Can we use to<HTMLFormElement> instead?
Chris Dumez
Comment 7 2014-12-12 19:59:00 PST
Comment on attachment 243153 [details] Reverted the scheme changes View in context: https://bugs.webkit.org/attachment.cgi?id=243153&action=review >> Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 >> + m_form = static_pointer_cast<HTMLFormElement>(element.release()); > > Can we use to<HTMLFormElement> instead? We ended up calling it downcast<> :) And if we use downcast, then we can get rid of the assertion above.
Darin Adler
Comment 8 2014-12-13 18:57:09 PST
Comment on attachment 243153 [details] Reverted the scheme changes View in context: https://bugs.webkit.org/attachment.cgi?id=243153&action=review >>> Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 >>> + m_form = static_pointer_cast<HTMLFormElement>(element.release()); >> >> Can we use to<HTMLFormElement> instead? > > We ended up calling it downcast<> :) > > And if we use downcast, then we can get rid of the assertion above. That’s funny, I can’t believe I forgot it was named downcast. Yes, we should use downcast here!
Ryosuke Niwa
Comment 9 2014-12-13 23:14:31 PST
(In reply to comment #8) > Comment on attachment 243153 [details] > Reverted the scheme changes > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243153&action=review > > >>> Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 > >>> + m_form = static_pointer_cast<HTMLFormElement>(element.release()); > >> > >> Can we use to<HTMLFormElement> instead? > > > > We ended up calling it downcast<> :) > > > > And if we use downcast, then we can get rid of the assertion above. > > That’s funny, I can’t believe I forgot it was named downcast. Yes, we should > use downcast here! How do I use downcast here given element is a PassRefPtr?
Chris Dumez
Comment 10 2014-12-14 00:44:17 PST
Comment on attachment 243153 [details] Reverted the scheme changes View in context: https://bugs.webkit.org/attachment.cgi?id=243153&action=review >>>>> Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 >>>>> + m_form = static_pointer_cast<HTMLFormElement>(element.release()); >>>> >>>> Can we use to<HTMLFormElement> instead? >>> >>> We ended up calling it downcast<> :) >>> >>> And if we use downcast, then we can get rid of the assertion above. >> >> That’s funny, I can’t believe I forgot it was named downcast. Yes, we should use downcast here! > > How do I use downcast here given element is a PassRefPtr? downcast<>() doesn't yet know of smart pointers. So right now, you would have to do: m_form = downcast<HTMLFormElement>(element.get()); Sadly this causes some ref-counting churn.
Chris Dumez
Comment 11 2014-12-14 00:54:13 PST
Comment on attachment 243153 [details] Reverted the scheme changes View in context: https://bugs.webkit.org/attachment.cgi?id=243153&action=review >>>>>> Source/WebCore/html/parser/HTMLConstructionSite.cpp:462 >>>>>> + m_form = static_pointer_cast<HTMLFormElement>(element.release()); >>>>> >>>>> Can we use to<HTMLFormElement> instead? >>>> >>>> We ended up calling it downcast<> :) >>>> >>>> And if we use downcast, then we can get rid of the assertion above. >>> >>> That’s funny, I can’t believe I forgot it was named downcast. Yes, we should use downcast here! >> >> How do I use downcast here given element is a PassRefPtr? > > downcast<>() doesn't yet know of smart pointers. So right now, you would have to do: > m_form = downcast<HTMLFormElement>(element.get()); > > Sadly this causes some ref-counting churn. The equivalent to static_pointer_cast<>() using downcast<>() right now would be: m_form = adoptRef(downcast<HTMLFormElement>(element.leakRef())); However, this isn't terribly readable.
Ryosuke Niwa
Comment 12 2014-12-14 02:24:35 PST
Comment on attachment 243153 [details] Reverted the scheme changes I'm gonna commit this as is. We can't fix static_pointer_cast later when we support downcast for PassRefPtr.
WebKit Commit Bot
Comment 13 2014-12-14 02:58:15 PST
Comment on attachment 243153 [details] Reverted the scheme changes Clearing flags on attachment: 243153 Committed r177263: <http://trac.webkit.org/changeset/177263>
WebKit Commit Bot
Comment 14 2014-12-14 02:58:21 PST
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.