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

RESOLVED FIXED in mozilla7

Status

()

Core
DOM
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: bz)

Tracking

Trunk
mozilla7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
See bug 652818 comment 6 (and subsequent discussion).
(Assignee)

Updated

6 years ago
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
(Assignee)

Comment 1

6 years ago
Created attachment 533766 [details] [diff] [review]
Just pass an JSContext to ReparentContentWrappersInScope.
Attachment #533766 - Flags: review?(peterv)
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]

Updated

6 years ago
Depends on: 659113
Created attachment 534559 [details] [diff] [review]
Rebased over bug 659113

Because I was feeling nice
Created attachment 534564 [details] [diff] [review]
Rebased over bug 659113

(This one should actually apply for you)
Attachment #534559 - Attachment is obsolete: true

Updated

6 years ago
Blocks: 659738
Created attachment 535843 [details] [diff] [review]
Pass it along
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+

Comment 6

6 years ago
http://hg.mozilla.org/mozilla-central/rev/f4273556fc42
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Whiteboard: [need review]

Updated

6 years ago
Target Milestone: --- → mozilla7
Version: Other Branch → Trunk
(Assignee)

Comment 7

6 years ago
May I ask for a favor?  Please don't check in my patches that have pending unaddressed review comments on them?  :(
(Assignee)

Comment 8

6 years ago
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!
(Assignee)

Comment 9

6 years ago
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).

Updated

6 years ago
Depends on: 660669
You need to log in before you can comment on or make changes to this bug.