The default bug view has changed. See this FAQ.

Remove nsIDOMGetSVGDocument

RESOLVED FIXED in mozilla24

Status

()

Core
SVG
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dzbarsky, Assigned: dzbarsky)

Tracking

({addon-compat, dev-doc-complete, site-compat})

unspecified
mozilla24
addon-compat, dev-doc-complete, site-compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
Created attachment 760273 [details] [diff] [review]
Patch
(Assignee)

Updated

4 years ago
Attachment #760273 - Attachment is patch: true
Attachment #760273 - Attachment mime type: message/rfc822 → text/plain
Attachment #760273 - Flags: review?(bzbarsky)
Comment on attachment 760273 [details] [diff] [review]
Patch

r=me
Attachment #760273 - Flags: review?(bzbarsky) → review+
This removes window.GetSVGDocument? If so, please fix test_interfaces.html
(Assignee)

Comment 3

4 years ago
(In reply to :Ms2ger from comment #2)
> This removes window.GetSVGDocument? If so, please fix test_interfaces.html

Right, didn't realize we don't actually run that test...
(Assignee)

Comment 4

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/efdf2d801664
Er, what do you mean we don't actually run it?
I think he means "after philor backs me out in https://hg.mozilla.org/integration/mozilla-inbound/rev/81b227f1a522 for causing test_interfaces.html to fail, then I'm going to know deep down in my bones that we do in fact run it, and that it will in fact fail when I land this patch."
Also seems to have busted a whole bunch of devtools browser-chrome tests.
(Assignee)

Comment 8

4 years ago
(In reply to Boris Zbarsky (:bz) from comment #5)
> Er, what do you mean we don't actually run it?

Looks like we don't run it on linux64.
Why *removing* an interface caused a failure of test_interfaces.html? Perhaps clobber was needed.
But anyway, we also need to remove use of window.GetSVGDocument from devtools.
BTW do we really want to remove GetSVGDocument? Even the latest editor's draft defines the interface.
https://dvcs.w3.org/hg/svg2/raw-file/tip/master/struct.html#InterfaceGetSVGDocument
We should convert it to WebIDL instead, no?
I don't think we want to remove getSVGDocument from HTMLObjectElement.prototype, etc., since it looks like people use it.  I think the interface in the spec should become [NoInterfaceObject], however.  Although Chrome does implement the interface, it doesn't expose it on the window.
(Better view of the SVG 2 draft here: https://svgwg.org/svg2-draft/struct.html#InterfaceGetSVGDocument)
(In reply to Cameron McCormack (:heycam) from comment #11)
> Although Chrome does implement the interface, it doesn't expose it on the
> window.

It's the same with IE 10 and Opera 12.15. Looks like we are the only browser exposing GetSVGDocument on the window.
> Looks like we don't run it on linux64.

Sure looks to me like we ran it on linux64 on your inbound push...

I do wonder why it failed, though.  It's possible that removing a .idl file needs a clobber or something dumb like that (e.g. if we don't properly remove .xpt files?)
Flags: needinfo?(khuey)
Flags: needinfo?(gps)
There are a number of open build config bugs around bad dependencies in XPIDL land. Hopefully these will all magically go away with bug 850380.
Flags: needinfo?(gps)
(In reply to David Zbarsky (:dzbarsky) from comment #3)
> (In reply to :Ms2ger from comment #2)
> > This removes window.GetSVGDocument? If so, please fix test_interfaces.html
> 
> Right, didn't realize we don't actually run that test...

We do, but it doesn't usually catch *removed* interfaces.
(Assignee)

Comment 17

4 years ago
Created attachment 761171 [details] [diff] [review]
Fix tests
Assignee: nobody → dzbarsky
Attachment #761171 - Flags: review?(bzbarsky)
Comment on attachment 761171 [details] [diff] [review]
Fix tests

>+      // then the node is a frame

No, you need to keep null-checking the return value of getSVGDocument().  Think <object> or <embed> showing an image or a plug-in...

r=me with that fixed.
Attachment #761171 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 19

4 years ago
(In reply to Boris Zbarsky (:bz) from comment #18)
> Comment on attachment 761171 [details] [diff] [review]
> Fix tests
> 
> >+      // then the node is a frame
> 
> No, you need to keep null-checking the return value of getSVGDocument(). 
> Think <object> or <embed> showing an image or a plug-in...
> 
> r=me with that fixed.

Ah, yes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/51369eb28a8f
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c813a569658
https://hg.mozilla.org/mozilla-central/rev/51369eb28a8f
https://hg.mozilla.org/mozilla-central/rev/0c813a569658
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_24
https://developer.mozilla.org/en-US/docs/Interfaces
Keywords: addon-compat, dev-doc-complete, site-compat
Flags: needinfo?(khuey)
You need to log in before you can comment on or make changes to this bug.