Closed Bug 606641 Opened 14 years ago Closed 13 years ago

Remove the DoInstantiateEmbeddedPlugin goop added in bug 484992

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla7

People

(Reporter: bzbarsky, Assigned: jaas)

Details

Attachments

(2 files)

fix v1.0
11.00 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
fix v2.0
6.83 KB, patch
jst
: review+
Details | Diff | Splinter Review 
See bug 484992 comment 21.  I think nowadays since we have no default plugin we should be able to just use SetUpPluginInstance directly.
Assignee: nobody → joshmoz
Attached patch fix v1.0 — — Splinter Review 

        Attachment #528836 -
        Flags: review?(bzbarsky)

    
    

    
  
This isn't quite what I was thinking when I filed this bug...  I was thinking we could rip out this boolean argument altogether.

    
    

    
  
Ah, I forgot about that. You mind if we take this cleanup first and I'll do that next?

    
    

    
  
Comment on attachment 528836 [details] [diff] [review]
fix v1.0

Sure.

        Attachment #528836 -
        Flags: review?(bzbarsky) → review+

    
    

    
  
pushed fix v1.0 to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/e9dafe775b31

    
  

        Attachment #528836 -
        Attachment description: fix v1.0 → cleanup v1.0

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix v2.0
        — 
          — Splinter Review
      

    


  

    
  

        Attachment #528836 -
        Attachment description: cleanup v1.0 → fix v1.0

    
  

        Attachment #534483 -
        Attachment description: fix v1.0 → fix v2.0

    
    

    
  
I'm not sure that fix v2.0 is really something we want yet, but I want to make sure that it addresses bz's concern, or is at least on the right track.

    
  

        Attachment #534483 -
        Flags: review?(jst)

    
    

    
  
Comment on attachment 534483 [details] [diff] [review]
fix v2.0

Since you want to know whether this addresses bz's concerns I think he should review here :)

Looks good to me fwiw, but I didn't dig too deep here.

        Attachment #534483 -
        Flags: review?(jst) → review?(bzbarsky)

    
    

    
  
I already told Josh that this addresses my concerns.  Now it needs correctness review, and I have no clue whether this is correct....  So I can't really review it.

    
    

    
  
Comment on attachment 534483 [details] [diff] [review]
fix v2.0

Ok, this looks correct to me. r=jst

        Attachment #534483 -
        Flags: review?(bzbarsky) → review+

    
    

    
  
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/7a6804f6034e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
  
Target Milestone: --- → mozilla7

    
    

    
  
Could anyone provide some clear Steps to reproduce in order to have this issue verified? Thanks.

    
  
Product: Core → Core Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: