Closed Bug 658213 Opened 11 years ago Closed 11 years ago

Use [implicit_jscontext] on document.open, delete cry for help in nsContentUtils::ReparentContentWrappersInScope

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: jorendorff, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 3 obsolete files)

See bug 652818 comment 6 (and subsequent discussion).
Summary: Use [explicit_jscontext] on document.open, delete cry for help in nsContentUtils::ReparentContentWrappersInScope → Use [implicit_jscontext] on document.open, delete cry for help in nsContentUtils::ReparentContentWrappersInScope
Attachment #533766 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
Depends on: 659113
Attached patch Rebased over bug 659113 (obsolete) — Splinter Review
Because I was feeling nice
Attached patch Rebased over bug 659113 (obsolete) — Splinter Review
(This one should actually apply for you)
Attachment #534559 - Attachment is obsolete: true
Attached patch Pass it alongSplinter Review
Attachment #533766 - Attachment is obsolete: true
Attachment #534564 - Attachment is obsolete: true
Attachment #533766 - Flags: review?(peterv)
Attachment #535843 - Flags: review?(peterv)
Comment on attachment 535843 [details] [diff] [review]
Pass it along

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

::: content/html/document/src/nsHTMLDocument.cpp
@@ +1545,5 @@
>  
>  // XXX TBI: accepting arguments to the open method.
>  nsresult
> +nsHTMLDocument::OpenCommon(JSContext *cx, const nsACString& aContentType,
> +                           PRBool aReplace)

It looks like there is only one caller of OpenCommon, can we remove OpenCommon and make this nsHTMLDocument::Open?

::: dom/interfaces/html/nsIDOMHTMLDocument.idl
@@ +39,5 @@
>  
>  #include "nsIDOMDocument.idl"
>  
> +%{C++
> +#include "jsapi.h"

Would jspubtd.h be better?
Attachment #535843 - Flags: review?(peterv) → review+
http://hg.mozilla.org/mozilla-central/rev/f4273556fc42
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [need review]
Target Milestone: --- → mozilla7
Version: Other Branch → Trunk
May I ask for a favor?  Please don't check in my patches that have pending unaddressed review comments on them?  :(
And to be clear, if I want someone to push one of my patches I will _definitely_ add the checkin-needed keyword to the bug!
Peter, I'll look into comment 5 tomorrow and probably land a followup...
Ms2ger told me he'd do the OpenCommon -> Open change in a followup (the jspubtd.h change got done already).
Depends on: 660669
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.