Closed Bug 533891 Opened 11 years ago Closed 11 years ago

remove default plugins

Categories

(Core :: Plug-ins, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: jaas, Assigned: jaas)

References

Details

Attachments

(1 file, 6 obsolete files)

I don't think we need default plugins any more. We should remove all of the default plugins and simply have display modes in the object frame - the object frame can just paint a missing plugin image when it doesn't have a plugin to display. We can add another display mode for a crashed plugin when we have OOP plugins.
Assignee: nobody → joshmoz
Attached patch fix v0.2 (obsolete) — Splinter Review
I suppose this takes care of bug 261751 too, yes?
We want this same thing for plugin-crashed UI. Josh says he wants to revise this patch to remove the nsObjectFrame if there isn't an available plugin and use a PFS-style binding mechanism even when PFS isn't enabled.
Blocks: 519541
Summary: kill default plugins → kill default plugins, introduce new placeholder UI mechanism
Should this be handled inside nsObjectFrame, or should it be a different frame class combined with new pseudo-classes driven by nsObjectLoadingContent?
(In reply to comment #4)
> Should this be handled inside nsObjectFrame, or should it be a different frame
> class combined with new pseudo-classes driven by nsObjectLoadingContent?

I have been discussing that with jst, we're probably going to do this from nsObjectLoadingContent, not nsObjectFrame.
Sounds good to me too.

Note that once we no longer have a default plug-in, we can possibly remove some hackery in nsObjectLoadingContent::OnStartRequest and certainly any consumers of nsObjectLoadingContent::ShouldShowDefaultPlugin (or rather any codepaths that follow on that returning true).
Attached patch kill default plugins, v1.0 (obsolete) — Splinter Review
This patch simply kills off default plugins and their associated handling code.
Attachment #416857 - Attachment is obsolete: true
Attached patch kill default plugins, v1.1 (obsolete) — Splinter Review
Attachment #420377 - Attachment is obsolete: true
Attachment #420391 - Attachment is patch: true
Attachment #420391 - Attachment mime type: application/octet-stream → text/plain
Blocks: 538910
Please update browser/installer/{package-manifest.in,removed-files.in} for files that are deprecated.
No longer blocks: 538910
Also: please remove plugin.default_plugin_disabled from firefox.js when this lands.
Attached patch fix v1.2 (obsolete) — Splinter Review
Updated to current mozilla-central. Does not remove the actual default plugin files to keep the patch manageable.
Attachment #420391 - Attachment is obsolete: true
Attached patch fix v1.3 (obsolete) — Splinter Review
Includes fixes for nsObjectLoadingContent.
Attachment #443946 - Attachment is obsolete: true
Attachment #443955 - Flags: review?(dolske)
Attached patch fix v1.4 (obsolete) — Splinter Review
added entries to removed-files.in
Attachment #443955 - Attachment is obsolete: true
Attachment #443963 - Flags: review?(dolske)
Attachment #443955 - Flags: review?(dolske)
Attachment #443963 - Flags: superreview?(jst)
Comment on attachment 443963 [details] [diff] [review]
fix v1.4

Yay! Glad to see this mess go! :)
Attachment #443963 - Flags: superreview?(jst) → superreview+
One question I ran into when I was working on bug 261751 was whether or not the lack of a plugins/ directory out of the box would potentially cause breakage. Does the installer need to create it if it doesn't exist?
Duplicate of this bug: 261751
I got this error on a Windows try server build, it appears to be a known intermittent orange (bug 453969) but I'm noting it here in case it is relevant in the future. I did not get the error on the Linux unit test run.

7363 INFO Running /tests/content/base/test/test_bug382113.html...
NEXT ERROR 7364 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug382113.html | Child got load event - got false, expected true
7365 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug382113.html | Object got load event - got false, expected true
Comment on attachment 443963 [details] [diff] [review]
fix v1.4

>-ifeq ($(OS_ARCH),OS2)
>-TOOL_DIRS += default/os2
>-endif

Nit: Might want to ping the OS/2 folks to see if whatever gets built for OS/2 needs added to the removed-files list.
Attachment #443963 - Flags: review?(dolske) → review+
Summary: kill default plugins, introduce new placeholder UI mechanism → remove default plugins
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/e074757a15aa
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
The changes here result in no firefox/plugins directory any more, which causes xpcshell (Xo & Xd on linux, at least) to fail like this

bash -c cp bin/xpcshell firefox && cp -R bin/components/* firefox/components/ && cp -R bin/plugins/* firefox/plugins/ && python -u xpcshell/runxpcshelltests.py --symbols-path=symbols --manifest=xpcshell/tests/all-test-dirs.list firefox/xpcshell
cp: cannot create regular file `firefox/plugins/': Is a directory

which is a really poor error message for 'firefox/plugins doesn't exist'. If we still support loading out of plugins/ then it would make sense to leave an empty directory there, and it would make the automation happy.

(This didn't show up on the original push because the build system doesn't clobber dist/firefox before packaging, so it took a clobber to get off the relbranch from Seamonkey's tagging to make it show up.)

Also, why didn't browser/installer/package-manifest.in get updated ?
Backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch fix v1.5Splinter Review
Includes fix for 'package-manifest.in'.
Attachment #443963 - Attachment is obsolete: true
Depends on: 564987
Depends on: 486570
(In reply to comment #21)
> which is a really poor error message for 'firefox/plugins doesn't exist'. If we
> still support loading out of plugins/ then it would make sense to leave an
> empty directory there, and it would make the automation happy.

There was some discussion about this in #developers and josh/bsmedberg decided against it. We'll work around this in Buildbot in the dependent bug.
Pushed to mozilla-central again now that bug 564987 is fixed.

http://hg.mozilla.org/mozilla-central/rev/558df3e5b094
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Had to back out again due to a talos master problem. Should be fixed by releng tomorrow.

http://hg.mozilla.org/mozilla-central/rev/dd3bac33fac2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
pushed to mozilla-central again after releng fixes applied

http://hg.mozilla.org/mozilla-central/rev/6a9d0521a319
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Blocks: 564656
Flags: in-testsuite-
Target Milestone: --- → mozilla1.9.3a5
Also need to fix up 
 http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-makefiles.sh#338
for this:
creating modules/plugin/default/mac/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/mac/Makefile.in: No such file or directory
creating modules/plugin/default/unix/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/unix/Makefile.in: No such file or directory
creating modules/plugin/default/windows/Makefile
can't read /builds/slave/mozilla-central-linux-nightly/build/modules/plugin/default/windows/Makefile.in: No such file or directory
There's a longstanding blanket-r+ from the build config peers to make obvious changes to allmakefiles.sh/toolkit-makefiles.sh.
Depends on: 703888
You need to log in before you can comment on or make changes to this bug.