678967 - Remove nsIDOMWindowInternal

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Comment 1

[Ed Morley [:emorley]]

Attachment: Patch v1

Removes nsIDOMWindowInternal again, now that it will have been depreciated for one cycle.

Is the backout of http://hg.mozilla.org/mozilla-central/rev/32c088e2048c (bug 675075), minus a couple of unrelated whitespace/#include re-ordering changes made then.

Returns UUIDs to their pre- bug 675075 values, since nothing else has landed since.

Presume a quick MXR search of AMO for usage of nsIDOMWindowInternal will be wanted before landing, to check that one cycle of being depreciated is enough.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think we're going to want to leave this around for more than 1 cycle ....

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I'm not sure why we'd want to do that. We should, IMO, make it clear that we do act on these deprecation warnings and that addressing them is not something to put off for a couple of years.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Why?  What's the ongoing cost of leaving this as is?

Comment 5

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

Comment on attachment 554038 [details] [diff] [review]
Patch v1


>-  nsIDOMWindow *caller = nsContentUtils::GetWindowFromCaller();
>+  nsIDOMWindow *caller =
>+    static_cast<nsIDOMWindow*>(nsContentUtils::GetWindowFromCaller());
why this change?


I think this could land soon. Addon devs have plenty of time to fix their code.
It will take still 12 weeks or so before FF8 is released. And 6 weeks from that before FF9 is released.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Olli Pettay [:smaug] from comment #5)
> Comment on attachment 554038 [details] [diff] [review]
> Patch v1
> 
> 
> >-  nsIDOMWindow *caller = nsContentUtils::GetWindowFromCaller();
> >+  nsIDOMWindow *caller =
> >+    static_cast<nsIDOMWindow*>(nsContentUtils::GetWindowFromCaller());
> why this change?

Because I claimed he could do a straight backout, and I forgot about that hunk.

Comment 7

[Ed Morley [:emorley]]

Attachment: Patch v1.1

v1 minus the unwanted static_cast<nsIDOMWindow*> hunk.

Comment 8

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #2)
> I think we're going to want to leave this around for more than 1 cycle ....

1 or 2 cycles where the warning is visible in the Error Console should be enough. If add-on developers are paying attention to console errors, they should notice within a single cycle. The rest won't notice until we remove it altogether, and there's nothing we can do about that.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I still don't understand what the point is.  We're breaking a bunch of addons to save 20 lines of code?  QIing to nsIDOMWindowInternal is all over MDC and the web in code samples for writing Firefox addons.

It seems like we're breaking backwards compatibility for no reason here.

Comment 10

[Jorge Villalobos [:jorgev] (he/him)]

I wouldn't say code cleanup is "no reason", but I would agree with you that it is not the most important of reasons. I don't really mind if the interface stays there forever to avoid breaking add-ons, but the backout patch included a warning indicating the interface is deprecated and will be removed. So, if we decide to keep the interface, then the warning should go.

Comment 11

[Ed Morley [:emorley]]

For whenever this lands (or doesn't) in the future; has passed try:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=f701ded4c303

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I still think this should be WONTFIX.  The cost of keeping this around is effectively 0.

Comment 13

[Ed Morley [:emorley]]

Once the next migration occurs (in just over a week), what do people want to do here?

1) Remove nsIDOMWindowInternal for Firefox 11 now that there will have been three cycles of depreciation notices (Firefox 8+9+10).

2) Leave nsIDOMWindowInternal in place, with the depreciation notice displaying - and close this bug as WONTFIX (or else leave the decision to 6 months down the line).

3) Leave nsIDOMWindowInternal in place indefinitely, and morph this bug into removing the depreciation notice, since it would no longer be accurate.

I don't really mind either way, I'd just quite like to resolve this bug and get it off my open-assigned saved search if possible :-)

Comment 14

[Jorge Villalobos [:jorgev] (he/him)]

I would prefer option 2, and comment #9 sums up the reasons pretty well. This is a too often used interface, and no matter what we do in terms of messaging, many add-ons will keep using it until we break it. We could revisit this decision in the future and see if it makes sense to remove the interface then, but it's hard to know what the success conditions for that would be, given that we have little knowledge of add-ons hosted outside of AMO, which are extremely popular at the moment.

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm going to WONTFIX this and hope nobody cares enough to reopen it.

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I don't think we should leave unnecessary cruft around in our code :)

Comment 17

[Ed Morley [:emorley]]

There are still quite a lot of MDN pages using nsIDOMWindowInternal in their examples. https://developer.mozilla.org/en-US/search?q=nsIDOMWindowInternal

Can they just be changed, or will they need the "up until version foo do this, otherwise do this" wiki template banner thing? I'm not overly familiar with how many versions back the examples/MDN docs tend to support...

Comment 18

[Benjamin Smedberg]

We need to balance our addon base against "nice to have" maintenance. In this case the addon community clearly wins. I don't think we should leave this bug open: it is an attractive nuisance and we would not *currently* accept a patch for it. File a new bug or reopen this one if we ever come up with data saying that these old extensions don't matter.

Comment 19

[Jean-Yves Perrier [:teoli]]

From the documentation point of view: even if the interface will stay for the foreseeable future, do we still want to keep it used in examples in the MDN or should we edit them in order not to use it anymore?

This should help prevent new code/add-ons start using it in the future.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Please remove it from examples on MDN.  .QueryInterface(Components.interfaces.nsIDOMWindowInternal) is useless no-op.

Comment 21

[Eric Shepherd [:sheppy]]

As far as I can tell, all docs that used to reference nsIDOMWindowInternal have been updated. If you find any, please point them out.