Merge https://chromium.googlesource.com/chromium/blink/+/45aadf7ee7ee010327eb692066cf013315ef3ed7 When parsing <form><template><input>, the previous behavior was to associate the <input> with the <form>, even though they're not in the same tree (or even the same document!). This patch changes that by checking, prior to creating a form control element, whether the element to be created lives in a document with a browsing context.
Created attachment 217392 [details] Patch
Created attachment 217393 [details] Patch
Comment on attachment 217393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217393&action=review > Source/WebCore/html/parser/HTMLConstructionSite.cpp:548 > - RefPtr<Element> element = HTMLElementFactory::createElement(tagName, ownerDocumentForCurrentNode(), form(), true); > + Document& ownerDocument = ownerDocumentForCurrentNode(); > + bool ownerDocumentHasBrowsingContext = !!ownerDocument.frame(); > + RefPtr<Element> element = HTMLElementFactory::createElement(tagName, ownerDocument, ownerDocumentHasBrowsingContext ? form() : 0, true); I'm not entirely sure this is the best way to do this. Shouldn't m_form be null if it does not apply in the current context? ownerDocumentHasBrowsingContext boolean seems unnecessary. 0 -> nullptr
(In reply to comment #3) > (From update of attachment 217393 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217393&action=review > > > Source/WebCore/html/parser/HTMLConstructionSite.cpp:548 > > - RefPtr<Element> element = HTMLElementFactory::createElement(tagName, ownerDocumentForCurrentNode(), form(), true); > > + Document& ownerDocument = ownerDocumentForCurrentNode(); > > + bool ownerDocumentHasBrowsingContext = !!ownerDocument.frame(); > > + RefPtr<Element> element = HTMLElementFactory::createElement(tagName, ownerDocument, ownerDocumentHasBrowsingContext ? form() : 0, true); > > I'm not entirely sure this is the best way to do this. Shouldn't m_form be null if it does not apply in the current context? > > ownerDocumentHasBrowsingContext boolean seems unnecessary. > 0 -> nullptr That's a good point. Let me dig into it.
Created attachment 217516 [details] Patch
We can't update m_form because HTMLTreeBuilder::processIsindexStartTagForInBody uses it. I've asked why this behavior is desirable for isindex on WHATWG: http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2013-November/041616.html If the spec. changes to also change is index's behavior we can match update our code accordingly. Until then, we should land this patch so that our behavior matches that of Chrome & Firefox.
Comment on attachment 217516 [details] Patch Attachment 217516 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/32028027 New failing tests: fast/forms/formmove3.html fast/parser/form-pointer-4.html fast/forms/preserveFormDuringResidualStyle.html fast/parser/form-pointer-1.html fast/parser/form-pointer-2.html fast/forms/parser-associated-form-removal.html fast/dom/HTMLTemplateElement/no-form-association.html
Created attachment 217517 [details] Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 217518 [details] Fixed the bug by negating the boolean logic
Comment on attachment 217518 [details] Fixed the bug by negating the boolean logic Clearing flags on attachment: 217518 Committed r159618: <http://trac.webkit.org/changeset/159618>
All reviewed patches have been landed. Closing bug.