Closed Bug 881128 Opened 7 years ago Closed 7 years ago

Remove nsIDOMGetSVGDocument

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: dzbarsky, Assigned: dzbarsky)

Details

(Keywords: addon-compat, dev-doc-complete, site-compat)

Attachments

(2 files)

Attached patch PatchSplinter Review
No description provided.
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
(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...
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.
(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.
(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.
Attached patch Fix testsSplinter Review
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+
(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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.