Closed Bug 1076836 Opened 5 years ago Closed 5 years ago

Stop storing a principal in txMozillaXSLTProcessor

Categories

(Core :: XSLT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: peterv, Assigned: peterv)

References

Details

Attachments

(2 files)

Attached patch v1Splinter Review
No description provided.
Attachment #8498841 - Flags: review?(bzbarsky)
Comment on attachment 8498841 [details] [diff] [review]
v1

r=me
Attachment #8498841 - Flags: review?(bzbarsky) → review+
FWIW, I think it'd be better to pass around an nsIDocument rather than an nsIPrincipal here. So that we could get both the principal and the loadgroup from that. And so that we could pass that document to NS_NewChannel when creating network channels.

But that could certainly be done as a separate bug.
I think that means you just volunteered to review, right?
Attachment #8499416 - Flags: review?(jonas)
Blocks: 842498
Comment on attachment 8499416 [details] [diff] [review]
Pass the document to TX_LoadSheet v1

Review of attachment 8499416 [details] [diff] [review]:
-----------------------------------------------------------------

Please also replace txCompileObserver::mLoadGroup with a txCompileObserver::mLoadingDocument. And remove txCompileObserver::mCallerPrincipal as it's unused (and should not be used).

::: dom/xslt/xslt/txMozillaStylesheetCompiler.cpp
@@ +532,5 @@
>          return NS_ERROR_DOM_BAD_URI;
>      }
>  
>      nsRefPtr<txCompileObserver> observer =
> +        new txCompileObserver(aProcessor, loadGroup);

I think we should be passing the document rather than the loadgroup here too.
Attachment #8499416 - Flags: review?(jonas) → review-
Comment on attachment 8499416 [details] [diff] [review]
Pass the document to TX_LoadSheet v1

Review of attachment 8499416 [details] [diff] [review]:
-----------------------------------------------------------------

I guess this patch is actually fine as-is. But we should get rid of the loadgroup and change it to a document as well. Though if you want to do that separately that's ok.

Sorry this took a while, I had to swap back this code into my brain.
Attachment #8499416 - Flags: review- → review+
https://hg.mozilla.org/mozilla-central/rev/1da35b1d5663
https://hg.mozilla.org/mozilla-central/rev/3e40213d4a21
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.