SVG-based custom element is removed from latest spec [1], we could remove related code from gecko as well. [1] https://www.w3.org/TR/custom-elements/

Revised patch description. Hi Edgar, may I have your feedback? Thank you.

Comment on attachment 8770021 [details] [diff] [review] Bug 1274505 - Remove SVG-based custom element support. v1 Need to remove svg testcase as well. Cancel feedback first.

Comment on attachment 8770052 [details] [diff] [review] Bug 1274505 - Remove SVG-based custom element support. v2 Review of attachment 8770052 [details] [diff] [review]: ----------------------------------------------------------------- Please also remove following lines from nsDocument.cpp: 1. https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/base/nsDocument.cpp#177 2. https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/base/nsDocument.cpp#195 3. https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/base/nsDocument.cpp#6406-6411 And for following lines in test_document_register.html. Although we can pass them, but I think we should still remove them. The exception is threw due to other reason, not the condition that we want to test. 1. https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/tests/mochitest/webcomponents/test_document_register.html#49 2. https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/tests/mochitest/webcomponents/test_document_register.html#99 ::: dom/base/nsDocument.cpp @@ -6390,5 @@ > } > > known = > ps->HTMLCaseSensitiveAtomTagToId(nameAtom) != eHTMLTag_userdefined; > - } else { namespace is always kNameSpaceID_XHTML after we remove svg supporting, so we can remove `if (namespaceID == kNameSpaceID_XHTML) {` as well. ::: dom/tests/mochitest/webcomponents/test_document_register.html @@ -72,5 @@ > testRegisterSimple("x-invalid-boolean", false, true); > testRegisterSimple("x-invalid-float", 1.0, true); > - // Can not register with a prototype that inherits from SVGElement > - // without extending an existing element type. > - testRegisterSimple("x-html-obj-svg", Object.create(SVGElement.prototype), true); Per offline discussion, I am surprised that you said you can not pass this test after removing SVG stuff. I thought registering an element whose prototype isn't inheriting from HTMLElement should throw an exception, but it not. Could you help to check why? And I think we should also add some tests for such case. @@ -149,1 @@ > var extendedText = document.createElementNS("http://www.w3.org/2000/svg", "text", "x-extended-text"); Remove this line. @@ -149,2 @@ > var extendedText = document.createElementNS("http://www.w3.org/2000/svg", "text", "x-extended-text"); > is(extendedText.tagName, "text", "Created element should have a local name of |text|."); Ditto. @@ -152,1 @@ > is(extendedText.getAttribute("is"), "x-extended-text", "The |is| attribute of the created element should be the extended type."); Ditto.

Comment on attachment 8770052 [details] [diff] [review] Bug 1274505 - Remove SVG-based custom element support. v2 Review of attachment 8770052 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsDocument.cpp @@ -6390,5 @@ > } > > known = > ps->HTMLCaseSensitiveAtomTagToId(nameAtom) != eHTMLTag_userdefined; > - } else { Will do. ::: dom/tests/mochitest/webcomponents/test_document_register.html @@ -72,5 @@ > testRegisterSimple("x-invalid-boolean", false, true); > testRegisterSimple("x-invalid-float", 1.0, true); > - // Can not register with a prototype that inherits from SVGElement > - // without extending an existing element type. > - testRegisterSimple("x-html-obj-svg", Object.create(SVGElement.prototype), true); Per offline discussion, the spec[1] said, "Interface objects that do not inherit from HTMLElement (for example TextTrack), or classes defined in the JavaScript specification (such as Set), will pass this step without throwing a TypeError. However, they will later cause create an element to fail". Since create an element will follow new spec, I think we don't need to add test for this bug. [1]https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregistry-define @@ -149,1 @@ > var extendedText = document.createElementNS("http://www.w3.org/2000/svg", "text", "x-extended-text"); Will do. @@ -149,2 @@ > var extendedText = document.createElementNS("http://www.w3.org/2000/svg", "text", "x-extended-text"); > is(extendedText.tagName, "text", "Created element should have a local name of |text|."); Will do. @@ -152,1 @@ > is(extendedText.getAttribute("is"), "x-extended-text", "The |is| attribute of the created element should be the extended type."); Will do.

Addressed comment 5.

(In reply to John Dai[:jdai] from comment #6) > Comment on attachment 8770052 [details] [diff] [review] > Bug 1274505 - Remove SVG-based custom element support. v2 > > Review of attachment 8770052 [details] [diff] [review]: > ----------------------------------------------------------------- > > Per offline discussion, the spec[1] said, "Interface objects that do not > inherit from HTMLElement (for example TextTrack), or classes defined in the > JavaScript specification (such as Set), will pass this step without throwing > a TypeError. However, they will later cause create an element to fail". > Since create an element will follow new spec, I think we don't need to add > test for this bug. > > [1]https://html.spec.whatwg.org/multipage/scripting.html#dom- > customelementsregistry-define Yes, you are right, thanks for clarifying this. :)

Add back non SVG-based custom element code.

try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec0ac06946d6e4f34986137729826be154e2e566&filter-tier=1

Comment on attachment 8771849 [details] [diff] [review] Bug 1274505 - Remove SVG-based custom element support. v3 Review of attachment 8771849 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/tests/mochitest/webcomponents/test_document_register.html @@ -58,5 @@ > // Registering without a prototype should automatically create one inheriting from HTMLElement. > testRegisterSimple("x-elem-no-proto", null, false); > - var simpleElem = document.createElement("x-elem-no-proto"); > - is(simpleElem.__proto__.__proto__, HTMLElement.prototype, "Default prototype should inherit from HTMLElement"); > - Nit: keep this blank line. @@ -154,2 @@ > // document.createElement with different namespace than definition for extended element. > var htmlText = document.createElement("text", "x-extended-text"); It seems we can remove `htmlText`, too.

Addressed comment 11.

Comment on attachment 8772330 [details] [diff] [review] Bug 1274505 - Remove SVG-based custom element support. v4 Review of attachment 8772330 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsDocument.cpp @@ +6367,3 @@ > } > > + known = You can move the declaration of |known| here.

Addressed comment 13. Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5bca8cd2b3c7c1031f9471f4fc3c98a7e19703c&filter-tier=1

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/930128b88444 Remove SVG-based custom element support. r=wchen