133642 - nsISelectionDisplay cannot be GetInterfaced from nsIWebBrowser.

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[mjudge]

This bug was fixed and checked into 9.8.  The last minute fix there did not get 
into the trunk. I am currently trying to check this fix into 9.9 and I need to 
check this into the trunk as well. This code is safe and tested in 9.8.  Simply 
a matter of the DocShell QI to look to its presshell and return the 
nsISelectionDisplay that is supports through its nsISelectionController 
interface.

Comment 1

[mjudge]

Attachment: fix left out of trunk from 9.8 checkin.

safe fix. need to support getinterface of embedders to change the seleciton
flags. we do this through nsISelectionDisplay.	9.8 had this change to the
GetInterface.  the trunk had all code in except these 2 snippets.

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 76338 [details] [diff] [review]
fix left out of trunk from 9.8 checkin.

r=bryner

Comment 3

[Simon Fraser [no longer active]]

Comment on attachment 76338 [details] [diff] [review]
fix left out of trunk from 9.8 checkin.

sr=sfraser

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 76338 [details] [diff] [review]
fix left out of trunk from 9.8 checkin.

a=roc+moz

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 76338 [details] [diff] [review]
fix left out of trunk from 9.8 checkin.

a=dbaron for trunk checkin.  However:

>+    }
>+    else if (aIID.Equals(NS_GET_IID(nsISelectionDisplay))) {
>+      nsCOMPtr<nsIPresShell> shell;
>+      nsresult rv = GetPresShell(getter_AddRefs(shell));
>+      if (NS_SUCCEEDED(rv) && shell)
>+        return shell->QueryInterface(aIID,aSink);

I think you need a |return NS_NOINTERFACE;| here -- otherwise you
risk attempting to addref and  returning NS_OK with an
**uninitialized** out param if GetPresShell fails or if the pres
shell is null.

>+  } else if (aIID.Equals(NS_GET_IID(nsISelectionDisplay))) {
>+    nsISelectionController* tmp = this;
>+    *aInstancePtr = (void*) tmp;

It seems a little more kosher to cast to |nsISelectionDisplay*|,
although this does work, since nsISelectionController derives
from nsISelectionDisplay.  (If there's multiple inheritance from
nsISelectionDisplay going on here, it might be good to make that
more explicit with a three-step cast.)

Comment 6

[dprice (gone)]

check on dbarons comments in 1.01

Comment 7

[Eli Friedman]

Resolving fixed, since apparently nobody got around to it.  (The code in question did in fact get checked in.)