112487 2013-03-15 18:06:35 -0700 [HTMLTemplateElement] prevent </template> from matching "template" in a non-HTML tags on the stack of open elements 2013-03-18 10:12:14 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 103547 1 rafaelw rafaelw abarth adamk eric esprehn+autocc ojan.autocc ojan webkit.review.bot oldest_to_newest 856451 0 rafaelw 2013-03-15 18:06:35 -0700 e.g. <template><svg><template><foreignObject><div></template> The </template> needs to clear back to the first (HTML) template, not the middle (SVG) template. Note that this is an open bug against HTML about this issue which exists in other contexts, and this fix is consistent with the most likely resolution (that the parser needs to be more aware about the namespace of elements when it trys to examine the stack of open elements for specific tags). 856454 1 193410 rafaelw 2013-03-15 18:09:18 -0700 Created attachment 193410 Patch 856464 2 193410 adamk 2013-03-15 18:29:46 -0700 Comment on attachment 193410 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193410&action=review > Source/WebCore/html/parser/HTMLTreeBuilder.cpp:968 > if (!m_tree.currentStackItem()->hasLocalName(token->name())) Not a big deal since it's "just" a parse error, but seems like hasTagName() would be better here. 856465 3 193410 rafaelw 2013-03-15 18:36:34 -0700 Comment on attachment 193410 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193410&action=review >> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:968 >> if (!m_tree.currentStackItem()->hasLocalName(token->name())) > > Not a big deal since it's "just" a parse error, but seems like hasTagName() would be better here. At this point, we know that |token| is an HTMLTemplate element, otherwise we wouldn't be here. I see what you are getting at, but it seems fine to me that HTML parser insertion modes can make the assumption that the current token is HTML. The danger we are fixing her is that the steps for processing foreignContent may have put non-HTML elements on the stack, so we have to be careful when examining it. 856775 4 193410 abarth 2013-03-17 16:52:15 -0700 Comment on attachment 193410 Patch Ok. Please make sure we stay in sync with the spec if this issue isn't resolved the way we expect. 856777 5 193410 webkit.review.bot 2013-03-17 17:00:52 -0700 Comment on attachment 193410 Patch Clearing flags on attachment: 193410 Committed r146028: <http://trac.webkit.org/changeset/146028> 856778 6 webkit.review.bot 2013-03-17 17:00:56 -0700 All reviewed patches have been landed. Closing bug. 857237 7 193410 adamk 2013-03-18 10:05:57 -0700 Comment on attachment 193410 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193410&action=review >>> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:968 >>> if (!m_tree.currentStackItem()->hasLocalName(token->name())) >> >> Not a big deal since it's "just" a parse error, but seems like hasTagName() would be better here. > > At this point, we know that |token| is an HTMLTemplate element, otherwise we wouldn't be here. I see what you are getting at, but it seems fine to me that HTML parser insertion modes can make the assumption that the current token is HTML. The danger we are fixing her is that the steps for processing foreignContent may have put non-HTML elements on the stack, so we have to be careful when examining it. I'm not sure how you can make the assumption that because |token| is being treated as HTML, that currentStackItem is also HTML. Perhaps because this is processTemplateEndTag? If it were a start tag, then you definitely couldn't make that assumption (since, e.g., <mtext> in MathML will cause the parser to treat tokens as HTML, and in that case, the currentStackItem will have a MathML namespace URI, though of course it won't have a <template> localName). Anyway, again, just a parse error, but I think we'll want to tighten up _all_ these callsites. 857242 8 193410 rafaelw 2013-03-18 10:12:14 -0700 Comment on attachment 193410 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193410&action=review >>>> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:968 >>>> if (!m_tree.currentStackItem()->hasLocalName(token->name())) >>> >>> Not a big deal since it's "just" a parse error, but seems like hasTagName() would be better here. >> >> At this point, we know that |token| is an HTMLTemplate element, otherwise we wouldn't be here. I see what you are getting at, but it seems fine to me that HTML parser insertion modes can make the assumption that the current token is HTML. The danger we are fixing her is that the steps for processing foreignContent may have put non-HTML elements on the stack, so we have to be careful when examining it. > > I'm not sure how you can make the assumption that because |token| is being treated as HTML, that currentStackItem is also HTML. Perhaps because this is processTemplateEndTag? If it were a start tag, then you definitely couldn't make that assumption (since, e.g., <mtext> in MathML will cause the parser to treat tokens as HTML, and in that case, the currentStackItem will have a MathML namespace URI, though of course it won't have a <template> localName). > > Anyway, again, just a parse error, but I think we'll want to tighten up _all_ these callsites. You are correct. Using hasTagName(templateTag) 193410 2013-03-15 18:09:18 -0700 2013-03-18 10:12:14 -0700 Patch bug-112487-20130315180513.patch text/plain 5696 rafaelw U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1OTcxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDc3NDFmMWVkYThmYmJi OTc0ZWM3OTlmZjM0YjE2OWQ5OGYwODVmNS4uNTRlYzMxNWE4MGM5OWZmYWVjYjc0MTE4YTRhZDUw N2VjYWYxMWVjOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI0IEBACisyMDEzLTAzLTE1ICBSYWZh ZWwgV2VpbnN0ZWluICA8cmFmYWVsd0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW0hUTUxUZW1w bGF0ZUVsZW1lbnRdIHByZXZlbnQgPC90ZW1wbGF0ZT4gZnJvbSBtYXRjaGluZyAidGVtcGxhdGUi IGluIGEgbm9uLUhUTUwgdGFncyBvbiB0aGUgc3RhY2sgb2Ygb3BlbiBlbGVtZW50cworICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEyNDg3CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2hlbiBwcm9jZXNzaW5n IGFuIGVuZCB0ZW1wbGF0ZSB0YWcsIHRoZSBwYXJzZXIgbm93IHBvcHMgdW50aWwgYSAidGVtcGxh dGUiIHRhZyBpcyBwYXJzZWQsIGJ1dCBub3cgZW5zdXJlcyB0aGF0CisgICAgICAgIHRoZSAidGVt cGxhdGUiIGl0IHBvcHMgaXMgaW4gdGhlIEhUTUwgbmFtZXNwYWNlLgorCisgICAgICAgIFRlc3Rz IGFkZGVkIHRvIHRoZSBodG1sNWxpYiB0ZXN0IHN1aXRlLgorCisgICAgICAgICogaHRtbC9wYXJz ZXIvSFRNTEVsZW1lbnRTdGFjay5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MRWxlbWVudFN0 YWNrOjpwb3BVbnRpbCk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAgICAgKFdlYkNvcmU6OkhU TUxFbGVtZW50U3RhY2s6OnBvcFVudGlsUG9wcGVkKToKKyAgICAgICAgKiBodG1sL3BhcnNlci9I VE1MRWxlbWVudFN0YWNrLmg6CisgICAgICAgIChIVE1MRWxlbWVudFN0YWNrKToKKyAgICAgICAg KiBodG1sL3BhcnNlci9IVE1MVHJlZUJ1aWxkZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRN TFRyZWVCdWlsZGVyOjpwcm9jZXNzVGVtcGxhdGVFbmRUYWcpOgorCiAyMDEzLTAzLTE1ICBEYW5h IEphbnNlbnMgIDxkYW5ha2pAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFtjaHJvbWl1bV0gUmVt b3ZlIHRoZSBiYWNrZ3JvdW5kIGZpbHRlciBibHVyIGxheW91dCB0ZXN0cwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTEVsZW1lbnRTdGFjay5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9odG1sL3BhcnNlci9IVE1MRWxlbWVudFN0YWNrLmNwcAppbmRleCBlM2FhNzhlYjMw NTIyNmJjOGExOTA2ZTIyZGU3ZjM4YmJlYzY3YWRiLi42YmI1ZmIyZWJiYjRmNjk0YmI2MTFiMDVh NmUyOTc1MWY4OWZmMTU1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9I VE1MRWxlbWVudFN0YWNrLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1M RWxlbWVudFN0YWNrLmNwcApAQCAtMjIwLDggKzIyMCwxNSBAQCB2b2lkIEhUTUxFbGVtZW50U3Rh Y2s6OnBvcCgpCiB2b2lkIEhUTUxFbGVtZW50U3RhY2s6OnBvcFVudGlsKGNvbnN0IEF0b21pY1N0 cmluZyYgdGFnTmFtZSkKIHsKICAgICB3aGlsZSAoIXRvcFN0YWNrSXRlbSgpLT5oYXNMb2NhbE5h bWUodGFnTmFtZSkpIHsKLSAgICAgICAgLy8gcG9wKCkgd2lsbCBBU1NFUlQgYXQgPGJvZHk+IGlm IGNhbGxlcnMgZmFpbCB0byBjaGVjayB0aGF0IHRoZXJlIGlzIGFuCi0gICAgICAgIC8vIGVsZW1l bnQgd2l0aCBsb2NhbE5hbWUgfHRhZ05hbWV8IG9uIHRoZSBzdGFjayBvZiBvcGVuIGVsZW1lbnRz LgorICAgICAgICAvLyBwb3AoKSB3aWxsIEFTU0VSVCBpZiBhIDxib2R5PiwgPGhlYWQ+IG9yIDxo dG1sPiB3aWxsIGJlIHBvcHBlZC4KKyAgICAgICAgcG9wKCk7CisgICAgfQorfQorCit2b2lkIEhU TUxFbGVtZW50U3RhY2s6OnBvcFVudGlsKGNvbnN0IFF1YWxpZmllZE5hbWUmIHRhZ05hbWUpCit7 CisgICAgd2hpbGUgKCF0b3BTdGFja0l0ZW0oKS0+aGFzVGFnTmFtZSh0YWdOYW1lKSkgeworICAg ICAgICAvLyBwb3AoKSB3aWxsIEFTU0VSVCBpZiBhIDxib2R5PiwgPGhlYWQ+IG9yIDxodG1sPiB3 aWxsIGJlIHBvcHBlZC4KICAgICAgICAgcG9wKCk7CiAgICAgfQogfQpAQCAtMjMyLDYgKzIzOSwx MiBAQCB2b2lkIEhUTUxFbGVtZW50U3RhY2s6OnBvcFVudGlsUG9wcGVkKGNvbnN0IEF0b21pY1N0 cmluZyYgdGFnTmFtZSkKICAgICBwb3AoKTsKIH0KIAordm9pZCBIVE1MRWxlbWVudFN0YWNrOjpw b3BVbnRpbFBvcHBlZChjb25zdCBRdWFsaWZpZWROYW1lJiB0YWdOYW1lKQoreworICAgIHBvcFVu dGlsKHRhZ05hbWUpOworICAgIHBvcCgpOworfQorCiB2b2lkIEhUTUxFbGVtZW50U3RhY2s6OnBv cFVudGlsTnVtYmVyZWRIZWFkZXJFbGVtZW50UG9wcGVkKCkKIHsKICAgICB3aGlsZSAoIXRvcFN0 YWNrSXRlbSgpLT5pc051bWJlcmVkSGVhZGVyRWxlbWVudCgpKQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTEVsZW1lbnRTdGFjay5oIGIvU291cmNlL1dlYkNvcmUv aHRtbC9wYXJzZXIvSFRNTEVsZW1lbnRTdGFjay5oCmluZGV4IDBiMzE5Mjg4Nzk3NjhjNmZmYTFi YmFiMDUwYjZmNzUxOTE3YTY4MzguLjViZTIxMWEzNjQ0N2Y4NTA4YzkyNjk4ZjQ1Y2Q0Y2EwMjE1 MmUwZDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxFbGVtZW50 U3RhY2suaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MRWxlbWVudFN0YWNr LmgKQEAgLTExNiw2ICsxMTYsMTEgQEAgcHVibGljOgogICAgIHZvaWQgcG9wVW50aWwoY29uc3Qg QXRvbWljU3RyaW5nJiB0YWdOYW1lKTsKICAgICB2b2lkIHBvcFVudGlsKEVsZW1lbnQqKTsKICAg ICB2b2lkIHBvcFVudGlsUG9wcGVkKGNvbnN0IEF0b21pY1N0cmluZyYgdGFnTmFtZSk7CisKKyAg ICAvLyBGSVhNRTogVGhlc2UgYXJlIGZpeGVzIGZvciBodHRwczovL3d3dy53My5vcmcvQnVncy9Q dWJsaWMvc2hvd19idWcuY2dpP2lkPTIxMjkyCisgICAgdm9pZCBwb3BVbnRpbChjb25zdCBRdWFs aWZpZWROYW1lJik7CisgICAgdm9pZCBwb3BVbnRpbFBvcHBlZChjb25zdCBRdWFsaWZpZWROYW1l Jik7CisKICAgICB2b2lkIHBvcFVudGlsUG9wcGVkKEVsZW1lbnQqKTsKICAgICB2b2lkIHBvcFVu dGlsTnVtYmVyZWRIZWFkZXJFbGVtZW50UG9wcGVkKCk7CiAgICAgdm9pZCBwb3BVbnRpbFRhYmxl U2NvcGVNYXJrZXIoKTsgLy8gImNsZWFyIHRoZSBzdGFjayBiYWNrIHRvIGEgdGFibGUgY29udGV4 dCIgaW4gdGhlIHNwZWMuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9I VE1MVHJlZUJ1aWxkZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRNTFRyZWVC dWlsZGVyLmNwcAppbmRleCA5NTMzMjliNzVkMGNjODVlODJmNzJiNTE5ODQ0ZTkwMjkyYmFkYjMy Li5iOGNjODQ4NTNiYTJhY2FmMDQzZmIxZWEwMmRiNmVlZmU4NDcyOGVjIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MVHJlZUJ1aWxkZXIuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL2h0bWwvcGFyc2VyL0hUTUxUcmVlQnVpbGRlci5jcHAKQEAgLTk2Nyw3ICs5Njcs NyBAQCBib29sIEhUTUxUcmVlQnVpbGRlcjo6cHJvY2Vzc1RlbXBsYXRlRW5kVGFnKEF0b21pY0hU TUxUb2tlbiogdG9rZW4pCiAgICAgbV90cmVlLmdlbmVyYXRlSW1wbGllZEVuZFRhZ3MoKTsKICAg ICBpZiAoIW1fdHJlZS5jdXJyZW50U3RhY2tJdGVtKCktPmhhc0xvY2FsTmFtZSh0b2tlbi0+bmFt ZSgpKSkKICAgICAgICAgcGFyc2VFcnJvcih0b2tlbik7Ci0gICAgbV90cmVlLm9wZW5FbGVtZW50 cygpLT5wb3BVbnRpbFBvcHBlZCh0b2tlbi0+bmFtZSgpKTsKKyAgICBtX3RyZWUub3BlbkVsZW1l bnRzKCktPnBvcFVudGlsUG9wcGVkKHRlbXBsYXRlVGFnKTsKICAgICBtX3RyZWUuYWN0aXZlRm9y bWF0dGluZ0VsZW1lbnRzKCktPmNsZWFyVG9MYXN0TWFya2VyKCk7CiAgICAgbV90ZW1wbGF0ZUlu c2VydGlvbk1vZGVzLnJlbW92ZUxhc3QoKTsKICAgICByZXNldEluc2VydGlvbk1vZGVBcHByb3By aWF0ZWx5KCk7CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKaW5kZXggYTc2MDVhMmNkYjlmZjJhMTI4YTg4MzBhMzViMzIzZDZjMzI3N2Ex OC4uMGI3N2FjY2ZkZjE0MTliZmRmNTJjNGMyOTk2ZmUxYmFiYWY0NjE5ZCAxMDA2NDQKLS0tIGEv TGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDEyIEBACisyMDEzLTAzLTE1ICBSYWZhZWwgV2VpbnN0ZWluICA8cmFmYWVsd0BjaHJvbWl1 bS5vcmc+CisKKyAgICAgICAgW0hUTUxUZW1wbGF0ZUVsZW1lbnRdIHByZXZlbnQgPC90ZW1wbGF0 ZT4gZnJvbSBtYXRjaGluZyAidGVtcGxhdGUiIGluIGEgbm9uLUhUTUwgdGFncyBvbiB0aGUgc3Rh Y2sgb2Ygb3BlbiBlbGVtZW50cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTEyNDg3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgKiBodG1sNWxpYi9yZXNvdXJjZXMvdGVtcGxhdGUuZGF0OgorCiAyMDEzLTAz LTE1ICBLZW5uZXRoIFJ1c3NlbGwgIDxrYnJAZ29vZ2xlLmNvbT4KIAogICAgICAgICBVbnJldmll d2VkIGFkZGl0aW9uIG9mIGV4cGVjdGF0aW9ucyBmb3IgdGVzdCBhZGRlZCBpbiByMTQ1OTE1Lgpk aWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvaHRtbDVsaWIvcmVzb3VyY2VzL3RlbXBsYXRlLmRhdCBi L0xheW91dFRlc3RzL2h0bWw1bGliL3Jlc291cmNlcy90ZW1wbGF0ZS5kYXQKaW5kZXggNjkxZDVl MDIzMDk2MzMzYTE2Y2FjYjQyYTMyNmY5ZGM4MTNiYzhkMC4uNjVhYTliNTg3YzlmNDMwYTA4YWU2 MTFhYmIyZDNjZTM1MjBmMjhkOCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvaHRtbDVsaWIvcmVz b3VyY2VzL3RlbXBsYXRlLmRhdAorKysgYi9MYXlvdXRUZXN0cy9odG1sNWxpYi9yZXNvdXJjZXMv dGVtcGxhdGUuZGF0CkBAIC0xMjMwLDMgKzEyMzAsMzEgQEAKIHwgICA8Ym9keT4KIHwgICAgIDxz cGFuPgogfCAgICAgICAiRm9vIgorCisjZGF0YQorPHRlbXBsYXRlPjxzdmc+PHRlbXBsYXRlPgor I2Vycm9ycworI2RvY3VtZW50Cit8IDxodG1sPgorfCAgIDxoZWFkPgorfCAgICAgPHRlbXBsYXRl PgorfCAgICAgICBjb250ZW50Cit8ICAgICAgICAgPHN2ZyBzdmc+Cit8ICAgICAgICAgICA8c3Zn IHRlbXBsYXRlPgorfCAgIDxib2R5PgorCisjZGF0YQorPHRlbXBsYXRlPjxzdmc+PGZvbz48dGVt cGxhdGU+PGZvcmVpZ25PYmplY3Q+PGRpdj48L3RlbXBsYXRlPjxkaXY+CisjZXJyb3JzCisjZG9j dW1lbnQKK3wgPGh0bWw+Cit8ICAgPGhlYWQ+Cit8ICAgICA8dGVtcGxhdGU+Cit8ICAgICAgIGNv bnRlbnQKK3wgICAgICAgICA8c3ZnIHN2Zz4KK3wgICAgICAgICAgIDxzdmcgZm9vPgorfCAgICAg ICAgICAgICA8c3ZnIHRlbXBsYXRlPgorfCAgICAgICAgICAgICAgIDxzdmcgZm9yZWlnbk9iamVj dD4KK3wgICAgICAgICAgICAgICAgIDxkaXY+Cit8ICAgPGJvZHk+Cit8ICAgICA8ZGl2Pgo=