1293786 - Remove SVGDocument

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

SVGDocument has been removed in SVG 2

Comment 1

[Robert Longson [:longsonr]]

SVG 1.1 https://www.w3.org/TR/SVG/struct.html#InterfaceGetSVGDocument
SVG 2   https://www.w3.org/TR/SVG2/struct.html#InterfaceGetSVGDocument

Chrome, Edge and Webkit have already done this.

Comment 2

[Robert Longson [:longsonr]]

Attachment: part 0 - fix existing mochitest

Comment 3

[Robert Longson [:longsonr]]

Attachment: part 1 - remove SVGDocument interface

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8779514 [details] [diff] [review]
part 1 - remove SVGDocument interface

Review of attachment 8779514 [details] [diff] [review]:
-----------------------------------------------------------------

r- but only because I think we need to expose rootElement on Document objects, as described in https://svgwg.org/svg2-draft/struct.html#InterfaceDocumentExtensions.  I'm can't test the WebKit change you reference (it doesn't seem to have made it into their Nightly build yet), but it looks like Chrome and Edge both have this implemented.  Chrome and Edge also both expose domain on both HTML and SVG documents, and the HTML spec puts domain on the Document interface, but we only have it on HTMLDocument.  I'm less worried about the lack of domain on SVG documents.  I tested with http://mcc.id.au/temp/root.html.

In terms of the actual type of document object SVG documents are exposed as to script, Edge uses Document and Chrome uses XMLDocument.  I'm fine with us following Chrome here.

Don't forget to get a DOM peer's review too, due to the IDL changes.

::: dom/webidl/moz.build
@@ -451,5 @@
>      'SVGClipPathElement.webidl',
>      'SVGComponentTransferFunctionElement.webidl',
>      'SVGDefsElement.webidl',
>      'SVGDescElement.webidl',
> -    'SVGDocument.webidl',

Please remove the file, too.

Comment 5

[Robert Longson [:longsonr]]

Don't know how to do that.

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Part 2: Move rootElement from the now-removed SVGDocument to Document.

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8779514 [details] [diff] [review]
part 1 - remove SVGDocument interface

With part 2 handling the rootElement moving, r=me on this patch with the other comments I made.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Ah, so this is just to remove the web exposing part of SVGDocument, not the implementation?
Why that?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8780401 [details] [diff] [review]
Part 2: Move rootElement from the now-removed SVGDocument to Document.

But I guess that is fine for now.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

What is the interface which svg documents now use? XMLDocument? What is it in other browsers?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

blink uses HTMLDocument, but looks like it isn't really HTMLdocument since things break easily when one tries to use stuff like document.write with it

Comment 12

[Cameron McCormack (:heycam)]

(In reply to Olli Pettay [:smaug] from comment #11)
> blink uses HTMLDocument, but looks like it isn't really HTMLdocument since
> things break easily when one tries to use stuff like document.write with it

From my testing in comment 4, Edge uses Document and Chrome uses XMLDocument.  Robert's patch makes us use XMLDocument.  And having just tested an updated WebKit Nightly, it too is using XMLDocument.  (But unlike other browsers it makes window.SVGDocument an alias for XMLDocument.)

Comment 13

[Cameron McCormack (:heycam)]

Comment on attachment 8779514 [details] [diff] [review]
part 1 - remove SVGDocument interface

Need DOM peer's review on this patch too.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8779514 [details] [diff] [review]
part 1 - remove SVGDocument interface

Isn't this missing to actually remove SVGDocument.webidl file?
Please 'hg remove' it

Comment 15

[Cameron McCormack (:heycam)]

See also https://gist.github.com/foolip/103963a1ae8598d2baedd296f4a1bf4cwhich Philip just wrote.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Why the patches haven't landed?

Comment 17

[Cameron McCormack (:heycam)]

One of us needs to update the patch to remove the now unused file; I'll look at that tomorrow.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c60775f445e
Part 0: Fix existing mochitest. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cba8a0a730c
Part 1: Remove SVGDocument interface. r=heycam,smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8911c81ee9a
Part 2: Move rootElement from the now-removed SVGDocument to Document. r=longsonr,smaug

Comment 19

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/fb604e5ff982 - yay unexpected passes in https://treeherder.mozilla.org/logviewer.html#?job_id=36676731&repo=mozilla-inbound but boo unexpected fails that directly want you not to have done that in https://treeherder.mozilla.org/logviewer.html#?job_id=36675866&repo=mozilla-inbound so wpt is going to need an Expectation Adjustment.

Comment 20

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee10e2a8f45d&group_state=expanded

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/63b988fc32fa
Part 0: Fix existing mochitest. r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/821c7f813eee
Part 1: Remove SVGDocument interface. r=heycam,smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7414e0e00acf
Part 2: Move rootElement from the now-removed SVGDocument to Document. r=longsonr,smaug

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63b988fc32fa
https://hg.mozilla.org/mozilla-central/rev/821c7f813eee
https://hg.mozilla.org/mozilla-central/rev/7414e0e00acf

Comment 23

[Kohei Yoshino [:kohei]]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/svgdocument-has-been-removed/

Comment 24

[Eric Shepherd [:sheppy]]

We never documented SVGDocument, so I updated the one page that sort of talked about it in a roundabout way to include information about the change and to clarify how to get documents in various conditions.

Updated:

https://developer.mozilla.org/en-US/docs/Web/SVG/Scripting#Inter-document_scripting:_referencing_embedded_SVG
https://developer.mozilla.org/en-US/Firefox/Releases/52#SVG

Please let me know if I’ve made any errors or left anything out.