252027 - [FIXr]Change content policy context back to nsISupports

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Upon some thought, some places really want to pass a window as a context to
content policy; the current DOMNode setup is a little too constraining.

So we should change this back to nsISupports and add a bit of documentation.

See bug 245836 for some of the discussion.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 154333 [details] [diff] [review]
Patch

It's kinda sad that I initially filed bug 191839  because the nsISupports was
being mistreated... let's hope that the documentation will prevent such
mistreatment in future.  :(

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 154333 [details] [diff] [review]
Patch

#if debug, should you make sure that the passed node can be QId to either
nsIDOMNode or nsIDOMWindow?  That might help curtail further abuse.

With or without that, as you deem appropriate, sr=shaver.  Can you mark this
for aviary landing when you commit (with an updated patch, if you make that
change)?  Thanks!

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, that's a good idea.  Will do.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 154333 [details] [diff] [review]
Patch

>Index: content/base/public/nsIContentPolicy.idl
>-   * @param aRequestingNode   OPTIONAL. the DOM node that initiated the
>+   * @param aContext          OPTIONAL. the DOM node or DOM Window that

Maybe document that it should be either a nsIDOMNode or nsIDOMWindow?

>Index: content/base/src/nsContentPolicy.h
>-                         nsIDOMNode *requestingNode,
>+                         nsISupports *requestingContext,

You seem to switch from context to requestingContext here (and in the rest of
the patch). Any reason for that?

r=mvl

Comment 6

[Boris Zbarsky [:bzbarsky]]

> You seem to switch from context to requestingContext here (and in the rest of
> the patch). Any reason for that?

Yes.  I didn't want to reindent all the comments in the IDL, so couldn't use
"aRequestingContext" there... ;)

I'll document that stuff should QI to nsIDOMNode or nsIDOMWindow.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch updated to comments

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 154426 [details] [diff] [review]
Patch updated to comments

Per shaver's request, asking for aviary approval.

Note that I can't check this in on aviary myself, so someone would need to do
that...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 154426 [details] [diff] [review]
Patch updated to comments

OK. a=asa. Shaver says it's cool.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Checked in on aviary by rjkeller.

Comment 12

[HJ]

Can I ask for Mozilla 1.7.6 approval in this bug report, just like comment #8
for the aviary branch ;)

Comment 13

[Boris Zbarsky [:bzbarsky]]

You can ask, but.....

It looks like bug 191839 was checked in on aviary (see bug 245280) but not on 1.7.  This 
bug depends on that one, if you want it to land on branch.  Ccing the people who did that 
backport to see whether that applies to 1.7, and ccing drivers to see whether they want 
this.

So much for api compat between Aviary and 1.7.5.... :(

Comment 14

[HJ]

(In reply to comment #13)
> You can ask, but.....
<snip/>
> Ccing the people who did that backport to see whether that applies to 1.7, and 
> ccing drivers to see whether they want this.

Thanks for the tip Boris, and I hope I won't forget this a next time.

Comment 15

[Mike Kaply [:mkaply]]

We miss stuff. It happens.

This is why aviary shouldn't have branched so early and more people should have
cared about 1.7 while aviary was being done.