Last Comment Bug 658213 - Use [implicit_jscontext] on document.open, delete cry for help in nsContentUtils::ReparentContentWrappersInScope
: Use [implicit_jscontext] on document.open, delete cry for help in nsContentUt...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla7
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 659113 660669
Blocks: 659738
  Show dependency treegraph
 
Reported: 2011-05-19 02:43 PDT by Jason Orendorff [:jorendorff]
Modified: 2011-05-30 09:59 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Just pass an JSContext to ReparentContentWrappersInScope. (14.93 KB, patch)
2011-05-19 13:08 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
Rebased over bug 659113 (11.91 KB, patch)
2011-05-23 14:14 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Rebased over bug 659113 (12.00 KB, patch)
2011-05-23 14:18 PDT, :Ms2ger (⌚ UTC+1/+2)
no flags Details | Diff | Splinter Review
Pass it along (11.97 KB, patch)
2011-05-28 04:25 PDT, :Ms2ger (⌚ UTC+1/+2)
peterv: review+
Details | Diff | Splinter Review

Description Jason Orendorff [:jorendorff] 2011-05-19 02:43:38 PDT
See bug 652818 comment 6 (and subsequent discussion).
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-05-19 13:08:18 PDT
Created attachment 533766 [details] [diff] [review]
Just pass an JSContext to ReparentContentWrappersInScope.
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2011-05-23 14:14:46 PDT
Created attachment 534559 [details] [diff] [review]
Rebased over bug 659113

Because I was feeling nice
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-05-23 14:18:28 PDT
Created attachment 534564 [details] [diff] [review]
Rebased over bug 659113

(This one should actually apply for you)
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2011-05-28 04:25:24 PDT
Created attachment 535843 [details] [diff] [review]
Pass it along
Comment 5 Peter Van der Beken [:peterv] 2011-05-30 04:00:40 PDT
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?
Comment 6 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-05-30 08:01:07 PDT
http://hg.mozilla.org/mozilla-central/rev/f4273556fc42
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-05-30 08:43:16 PDT
May I ask for a favor?  Please don't check in my patches that have pending unaddressed review comments on them?  :(
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2011-05-30 08:43:55 PDT
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!
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-05-30 08:44:20 PDT
Peter, I'll look into comment 5 tomorrow and probably land a followup...
Comment 10 Peter Van der Beken [:peterv] 2011-05-30 09:06:06 PDT
Ms2ger told me he'd do the OpenCommon -> Open change in a followup (the jspubtd.h change got done already).

Note You need to log in before you can comment on or make changes to this bug.