Closed Bug 303815 Opened 19 years ago Closed 19 years ago

make plugin scriptability check work on x86 Macs and with Universal Binaries

Categories

(Core Graveyard :: Plug-ins, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jaas, Assigned: jaas)

Details

Attachments

(1 file, 1 obsolete file)

Make plugin scriptability check work on x86 Macs and with Universal Binaries.
Right now it only considers 32-bit thin mach-o PPC binaries to be scriptable.
Attached patch fix v1.0 (obsolete) — Splinter Review
This should fix the problem. However, I don't know of a way to test it. Any
ideas?

Two things to improve in the future, after this patch: consider moving Mac OS X
executable header checks into shared functions and account for the possibility
of FAT binaries not including code for the current arch. Those would be nice,
but are not critical at the moment.
Attachment #191915 - Flags: superreview?(sfraser_bugs)
Attachment #191915 - Flags: review?(mark)
Comment on attachment 191915 [details] [diff] [review]
fix v1.0

cancelling reviews as I found a problem
Attachment #191915 - Flags: superreview?(sfraser_bugs)
Attachment #191915 - Flags: review?(mark)
Attached patch fix v1.1Splinter Review
Attachment #191915 - Attachment is obsolete: true
Attachment #191916 - Flags: superreview?(sfraser_bugs)
Attachment #191916 - Flags: review?(mark)
Comment on attachment 191916 [details] [diff] [review]
fix v1.1

To test this you need a UB plugin that does scripting stuff (look for the
tell-tale .xpt file), and a web page that has JS to script the plugin. Maybe
look around on the quicktime site.
Attachment #191916 - Flags: superreview?(sfraser_bugs) → superreview+
Comment on attachment 191916 [details] [diff] [review]
fix v1.1

Your changes look good, but I'm minusing because of a preexisting error in the
code.  open_executable can return either 0 or -1 on failure: 0 if any of its
own checks fail, and -1 if it calls open and open fails.  The (fd) check in
IsCompatibleExecutable should check for positiveness, or open_executable should
be "fixed."

The comment at the top of the relevant XP_MACOSX section should be updated now
that we're not just checking for raw Mach-O and not using hard-coded FEEDFACE.
Attachment #191916 - Flags: review?(mark) → review-
Comment on attachment 191916 [details] [diff] [review]
fix v1.1

Josh says he'll fix on checkin.
Attachment #191916 - Flags: review- → review+
Comment on attachment 191916 [details] [diff] [review]
fix v1.1

Requesting approval. I'll change the check to:

if (fd > 0)
Attachment #191916 - Flags: approval1.8b4?
Attachment #191916 - Flags: approval1.8b4? → approval1.8b4+
landed with file descriptor check and comment fixes
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
This check-in make bustage on prometheus.

> nsPluginHostImpl.cpp: In constructor `nsPluginTag::nsPluginTag(nsPluginInfo*)':
> nsPluginHostImpl.cpp:878: error: 'struct nsPluginInfo' has no member named '
>    fBundle'

it has been fixed
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: