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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

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.

Comment 1

[Josh Aas]

Attachment: fix v1.0

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.

Comment 2

[Josh Aas]

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

cancelling reviews as I found a problem

Comment 3

[Josh Aas]

Attachment: fix v1.1

Comment 4

[Simon Fraser [no longer active]]

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.

Comment 5

[Mark Mentovai]

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.

Comment 6

[Mark Mentovai]

Comment on attachment 191916 [details] [diff] [review]
fix v1.1

Josh says he'll fix on checkin.

Comment 7

[Josh Aas]

Comment on attachment 191916 [details] [diff] [review]
fix v1.1

Requesting approval. I'll change the check to:

if (fd > 0)

Comment 8

[Josh Aas]

landed with file descriptor check and comment fixes

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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'

Comment 10

[Josh Aas]

it has been fixed