Closed Bug 1216885 Opened 9 years ago Closed 7 years ago

warning: interface 'nsISelectionPrivate' is scriptable but derives from non-scriptable 'nsISelection', ../../../dist/idl/nsISelectionPrivate.idl

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox44 --- affected
firefox57 --- fixed

People

(Reporter: dholbert, Assigned: mccr8)

References

Details

Attachments

(1 file)

Just ran across this build warning:
{
warning: interface 'nsISelectionPrivate' is scriptable but derives from non-scriptable 'nsISelection', ../../../dist/idl/nsISelectionPrivate.idl line 31:0
interface nsISelectionPrivate : nsISelection
^
}

(The warning is present in TreeHerder logs, too -- it's not just my local machine.  e.g. this one I grabbed from recent a mozilla-central run:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-linux64-debug/1445386861/mozilla-central-linux64-debug-bm72-build1-build111.txt.gz
)

I'm guessing we've been spamming this warning since mid-2014 when we made nsISelection non-scriptable in bug 994964:
http://hg.mozilla.org/mozilla-central/diff/c590fcb23c3c/content/base/public/nsISelection.idl

ehsan, should we have dropped "scriptable" from nsISelectionPrivate there as well?
Flags: needinfo?(ehsan)
OK -- any suggestions on how to silence / address this warning, then?

(I think this may be our only instance of an "is scriptable but derives from non-scriptable" warning, BTW; it's the only instance in the TreeHerder log linked in comment 0, at least.)
Blocks: 337723
Kyle, what purpose does this warning serve?
Flags: needinfo?(khuey)
bz may be able to offer some insights, too.

He said in bug 337723 comment 4 (10 years ago):
> In general, imo xpidl should error, not warn, when a scriptable interface inherits from a non-scriptable one.

That makes it sound like this warning is worth worrying about. But I don't really know.
Well if you actually care about the IDL inheritance, you should care about it from script too, no?  And if you don't, just break the inheritance chain.
Flags: needinfo?(khuey)
What Kyle said.  The inheritance chain either makes sense or doesn't.  If it makes sense, then it needs to actually make sense; if not, there's no reason for the interfaces to inherit from each other.

The problem is C++ developers who don't realize they're creating something nonsensical from a JS point of view.
So I guess in this case we want to make nsISelectionPrivate not inherit from nsISelection.  That may end up being a good chunk of work, I'm not going to do that myself, but I'd r+ a patch.  :-)
hi,
I'm trying to compile and I get the same error.
I can not find the patch?
:Ehsan can you tell me where the patch is located
because I want to make demos of upcoming events where Mozilla has a stand
thank you

Christophe (:hellosct1)
Flags: needinfo?(ehsan)
Sorry for the late response, Christophe, I was out on vacation/sick leave for a while.

As far as I know, no patch has been written for this bug.  What I was trying to say in comment 7 was that if someone else writes a patch, I'd be happy to review it. :-)
Flags: needinfo?(ehsan)
In addition to what ehsan said:
(In reply to :hellosct1 from comment #8)
> I'm trying to compile and I get the same error.

This build warning should not actually cause your build to fail - this is just a build warning, not an error.  (It's just noise during the build process.)

If your build was failing and you noticed this warning in the output, then it's likely there's another build error hiding somewhere else in the output, and that's the real source of your trouble.
I happened across this error. It just seems like implementing Ehsan's suggestion from comment 7 just works, at least assuming we can trust try: try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=88269702e9f26e562231728f520a04509cf2fa5b

I'll need to read through some code a bit more to convince myself.
Assignee: nobody → continuation
I looked through all of the places that use nsISelectionPrivate. Most of them QI to it, then only call methods from Private on it, so that works just fine.

layout/base/tests/test_scroll_selection_into_view.html calls methods from both nsISelection and nsISelectionPrivate:

  var selection = SpecialPowers.wrap(win.getSelection())
                               .QueryInterface(SpecialPowers.Ci.nsISelectionPrivate);
  ...
  if (target.contentDocument) {
    selection = SpecialPowers.wrap(target.contentWindow.getSelection())
                             .QueryInterface(SpecialPowers.Ci.nsISelectionPrivate);
    ...
  }
  selection.collapse(target.parentNode, 0); // method on nsISelection
  ...
  selection.scrollIntoView(FOCUS, true, vPercent, 0); // method on nsISelectionPrivate

This may work okay because we initially have an object that implements nsISelection, so when we QI it to the Private interface, we end up with an object that implements both. I'm not entirely sure. But the test still passes.
I guess it is axiomatic that no script depends on this, because the reason for the warning is that script can't see the inheritance...
Blocks: 338865
Comment on attachment 8907307 [details]
Bug 1216885 - Make nsISelectionPrivate not inherit from nsISelection.

https://reviewboard.mozilla.org/r/178972/#review184100
Attachment #8907307 - Flags: review?(bugs) → review+
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ca25d95e1aa
Make nsISelectionPrivate not inherit from nsISelection. r=smaug
https://hg.mozilla.org/mozilla-central/rev/1ca25d95e1aa
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.