Last Comment Bug 598397 - remove support for Carbon NPAPI
: remove support for Carbon NPAPI
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla19
Assigned To: Josh Aas
:
Mentors:
Depends on:
Blocks: 787510 753251 790373 850925
  Show dependency treegraph
 
Reported: 2010-09-21 11:47 PDT by Josh Aas
Modified: 2013-03-14 01:46 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1.0 (87.15 KB, patch)
2012-10-12 07:48 PDT, Josh Aas
smichaud: review+
Details | Diff | Splinter Review

Description Josh Aas 2010-09-21 11:47:23 PDT
https://mail.mozilla.org/pipermail/plugin-futures/2010-June/000120.html

The Carbon event model is considered deprecated in Firefox 4 (Gecko 2.0.0) and we are planning to remove support for it entirely after Firefox 4. Its replacement is the Cocoa event model, introduced in Firefox 4 (Gecko 2.0.0). We are working with plugin vendors to make sure they are ready for this change.

Reasoning:

1. The Carbon event model is generally inferior to the Cocoa event model. It also isn't as well documented.

2. Our out-of-process plugin implementation does not support the Carbon event model and we want to require all plugins to run out of process by default. The Carbon event model was not designed with IPC in mind, the Cocoa event model was.

3. Maintaining multiple/duplicate event models in our code significantly increases complexity which results in slower development and QA, and ultimately lower quality.

4. We are moving towards x86_64 as our primary architecture for Mac OS X. Implementing Carbon NPAPI support in our x86_64 code may not be impossible but it would almost certainly be an unreasonable endeavor.

5. Removing support entirely is the ultimate form of deterrence for plugin vendors. We really don't want people shipping Carbon event model plugins after Firefox 4 is released.
Comment 1 Josh Aas 2011-02-20 15:43:41 PST
I think we need to keep Carbon NPAPI support so long as we support Mac OS X 10.5 so this probably won't happen for the release after Firefox 4.
Comment 2 Stuart Morgan 2012-08-28 22:01:56 PDT
FYI, in case parity is a factor in when this lands: Starting with Chrome 22, Mac Chrome will not support the Carbon event model (we're dropping it in parallel with the QuickDraw drawing model).
Comment 3 Stuart Morgan 2012-08-28 22:07:57 PDT
I should also explain the motivation: the *only* plugin that I'm aware of that uses CG+Carbon at this point is Garmin, and they have a 64-bit plugin using CG+Cocoa so anyone running Firefox in 64-bit mode wouldn't see any change. And since the plugin is broken now in Chrome (which I've communicated to them), I expect them to update at some point fairly soon to do event negotiation in 32-bit mode.

So it's likely that once bug 598401 lands there is essentially zero user impact to doing this too.
Comment 5 Steven Michaud [:smichaud] (Retired) 2012-10-12 12:59:17 PDT
Looks fine to me.

One small nit:

The definition of kFocusedChildViewTSMDocPropertyTag and the calls to TSMSetDocumentProperty() and TSMRemoveDocumentProperty() that surround the single remaining call to TSMProcessRawKeyEvent() are no longer needed.  They were used to pass information to code (in PluginTextInputHandler::HandleKeyUpEvent()) that your patch removes.
Comment 6 Josh Aas 2012-10-12 13:09:21 PDT
Also, if we want to do a phase two we can probably remove a lot more checks for the Cocoa event model since it is the only model we support now. I left a bunch of checks in to simplify the patch.
Comment 7 Steven Michaud [:smichaud] (Retired) 2012-10-12 14:06:58 PDT
> One small nit:
>
> The definition of kFocusedChildViewTSMDocPropertyTag and the calls
> to TSMSetDocumentProperty() and TSMRemoveDocumentProperty() that
> surround the single remaining call to TSMProcessRawKeyEvent() are no
> longer needed.  They were used to pass information to code (in
> PluginTextInputHandler::HandleKeyUpEvent()) that your patch removes.

Oops.  I just double-checked and this is wrong.

kFocusedChildViewTSMDocPropertyTag is also checked in
PluginTextInputHandler::PluginKeyEventsHandler(), which your patch
doesn't remove.
Comment 8 Josh Aas 2012-10-15 04:05:57 PDT
Another try server run with different platforms:

https://tbpl.mozilla.org/?tree=Try&rev=cfa1565942da
Comment 10 Ed Morley [:emorley] 2012-10-16 01:26:52 PDT
https://hg.mozilla.org/mozilla-central/rev/faa749453ea4

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