223192 - content/ could use nsContentUtils::GetSecurityManager() instead of pegging the service manager

Status: RESOLVED FIXED

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

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

Given the comment and code at
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsContentUtils.cpp#104
we need null-checks in a bunch of places if you decide to do this....

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Re-trying for reviews.	This patch is actually right, but the code you pointed
out sucks.  I filed bug 223373 to fix that and won't land this without that
landing first, but this can get reviews in the mean time.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

>Index: content/base/src/nsDocument.cpp
>+    nsIScriptSecurityManager* securityManager =
>+      nsContentUtils::GetSecurityManager();
>+    rv = securityManager->CheckLoadURI(mDocumentURL, aURL,

Do you even need the temporary?  I guess it makes short lines easier...

Same in a few other places.

>Index: content/html/content/src/nsHTMLFormElement.cpp
>   // XXX This code has not been tested.  mailto: does not work in forms.
>   //

That XXX comment is bogus.  Could you just remove it?

r+sr=bzbarsky with that (do whatever you want about the temporaries; I don't
care, really).

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Yeah, I used the temporaries on the longer lines since IMO they make the code
much easier to read.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

(and much easier to keep under 80 chars)

Fix checked in.