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)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla7
People
(Reporter: bzbarsky, Assigned: jaas)
Details
Attachments
(2 files)
|
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
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.
Attachment #528836 -
Flags: review?(bzbarsky)
|
Reporter | |
Comment 2•14 years ago
|
||
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?
|
|
Reporter | |
Comment 4•14 years ago
|
||
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
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 8•13 years ago
|
||
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)
|
|
Reporter | |
Comment 9•13 years ago
|
||
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 10•13 years ago
|
||
Comment on attachment 534483 [details] [diff] [review]
fix v2.0
Ok, this looks correct to me. r=jst
Attachment #534483 -
Flags: review?(bzbarsky) → review+
|
Assignee | |
Comment 11•13 years ago
|
||
pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/7a6804f6034e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
|
||
Updated•13 years ago
|
Target Milestone: --- → mozilla7
|
||
Comment 12•13 years ago
|
||
Could anyone provide some clear Steps to reproduce in order to have this issue verified? Thanks.
|
||
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•