remove support for Quickdraw NPAPI

RESOLVED FIXED in mozilla18

Status

()

Core
Plug-ins
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
mozilla18
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Assignee)

Description

7 years ago
https://mail.mozilla.org/pipermail/plugin-futures/2010-June/000120.html

The Quickdraw drawing model is considered deprecated and we are planning to remove support for it entirely after Firefox 4. We recommend that plugin vendors use the Core Graphics or Core Animation drawing models. We are working with plugin vendors to make sure they are ready for this change.

Also, see bug 598397 about removing the Carbon event model.
(Assignee)

Comment 1

6 years ago
Created attachment 527495 [details] [diff] [review]
fix v1.0
(Assignee)

Comment 2

6 years ago
Created attachment 527565 [details] [diff] [review]
fix v1.1
Attachment #527495 - Attachment is obsolete: true
Attachment #527565 - Flags: review?(smichaud)
Comment on attachment 527565 [details] [diff] [review]
fix v1.1

This looks fine to me.  But I'm puzzled you didn't remove all
references to NP_NO_QUICKDRAW, even from the files you changed.

With the exception of npapi.h -- I *don't* think we should remove
NP_NO_QUICKDRAW from that file, because it's used by other vendors.

Here are the other places (in current trunk code) where you didn't
remove NP_NO_QUICKDRAW, by file name.  I list first the files that you
changed in this patch.

layout/generic/nsObjectFrame.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/layout/generic/nsObjectFrame.cpp#l3223
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/layout/generic/nsObjectFrame.cpp#l4284

modules/plugin/base/src/nsNPAPIPlugin.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/modules/plugin/base/src/nsNPAPIPlugin.cpp#l2197
(Here you did change the return value to PR_FALSE.)

widget/src/cocoa/nsChildView.mm:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/widget/src/cocoa/nsChildView.mm#l2308

dom/plugins/ipc/PluginInstanceChild.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/dom/plugins/ipc/PluginInstanceChild.cpp#l430

modules/plugin/base/src/nsNPAPIPluginInstance.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/modules/plugin/base/src/nsNPAPIPluginInstance.cpp#l72

widget/src/xpwidgets/nsBaseWidget.cpp:
http://hg.mozilla.org/mozilla-central/annotate/863db843fde4/widget/src/xpwidgets/nsBaseWidget.cpp#l790
Attachment #527565 - Flags: review?(smichaud) → review+
(Assignee)

Comment 4

6 years ago
Almost all of those cases (like the ones in nsObjectFrame) are i386 default Quickdraw values which are part of the API - they don't change when we don't support Quickdraw. We need to detect whether or not that has been negotiated by the plugin. Same goes for where we tell the plugin we don't support Quickdraw - in order to use the Quickdraw constants we need NP_NO_QUICKDRAW.

We probably should change the nsBaseWidget to a !i386 macro. I'll do that in a new patch. Thanks for the review.
(Assignee)

Comment 5

6 years ago
Created attachment 527917 [details] [diff] [review]
fix v1.2
Attachment #527565 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Assignee: joshmoz → nobody
(Assignee)

Updated

5 years ago
Assignee: nobody → joshmoz
(Assignee)

Comment 6

5 years ago
Created attachment 641099 [details] [diff] [review]
fix v1.3

Not tested extensively yet.
Attachment #527917 - Attachment is obsolete: true
Attachment #641099 - Flags: review?(smichaud)
(Assignee)

Comment 7

5 years ago
I'm planning to do this for Firefox 17, when we will no longer support Mac OS X 10.5.
Comment on attachment 641099 [details] [diff] [review]
fix v1.3

While basically OK, this has some problems:

This patch drops support for printing plugins on OS X.  Will restoring
it be the subject of another bug?  We presumably shouldn't land this
patch until we've figured out how to print plugins without the few
QuickDraw calls we've been using.

+  if (!mView) {
     // [NSGraphicsContext currentContext] is supposed to "return the
     // current graphics context of the current thread."  But sometimes
     // (when called while mView isn't focused for drawing) it returns a
     // graphics context for the wrong window.  [window graphicsContext]
     // (which "provides the graphics context associated with the window
     // for the current thread") seems always to return the "right"
     // graphics context.  See bug 500130.
     mPluginCGContext.context = NULL;
     mPluginCGContext.window = NULL;
 #ifndef NP_NO_CARBON
+    NSWindow* cocoaWindow = [mView window];
+    WindowRef carbonWindow = cocoaWindow ? (WindowRef)[cocoaWindow windowRef] : NULL;
     if (carbonWindow) {
       mPluginCGContext.context = (CGContextRef)[[cocoaWindow graphicsContext] graphicsPort];
       mPluginCGContext.window = carbonWindow;
     }
 #endif
   }

The "if (!mView) {}" that surrounds this block should be dropped.

-// In QuickDraw mode the coordinate system used here should be that of the
-// browser window's content region (defined as everything but the 22-pixel
-// high titlebar).  But in CoreGraphics mode the coordinate system should be
-// that of the browser window as a whole (including its titlebar).  Both
-// coordinate systems have a top-left origin.  See bmo bug 474491.
+// In CoreGraphics mode the coordinate system should be that of the browser
+// window as a whole (including its titlebar).  This coordinate system has a
+// top-left origin.  See bmo bug 474491.

Without the description of the QuickDraw coordinate system, this whole
comment makes a lot less sense.

-  // In QuickDraw drawing mode, prevent reentrant handling of any plugin event
-  // (this emulates behavior on the 1.8 branch, where only QuickDraw mode is
-  // supported).  But in CoreGraphics drawing mode only do this if the current
+  // In CoreGraphics drawing mode only do this if the current

Same thing again.

But since we're no longer using QuickDraw, maybe this should be
rephrased as follows:

"In QuickDraw drawing mode we used to prevent reentrant handling of
any plugin event.  But in CoreGraphics drawing mode only do this if
the current ..."

-  // 10.6.2 and lower have a bug involving textures and pixel buffer objects
-  // that caused bug 629016, so we don't allow OpenGL-accelerated layers on
-  // those versions of the OS.
-  // This will still let full-screen video be accelerated on OpenGL, because
-  // that XUL widget opts in to acceleration, but that's probably OK.
-  SInt32 major, minor, bugfix;
-  OSErr err1 = ::Gestalt(gestaltSystemVersionMajor, &major);
-  OSErr err2 = ::Gestalt(gestaltSystemVersionMinor, &minor);
-  OSErr err3 = ::Gestalt(gestaltSystemVersionBugFix, &bugfix);
-  if (err1 == noErr && err2 == noErr && err3 == noErr) {
-    if (major == 10 && minor == 6) {
-      if (bugfix <= 2) {
-        accelerateByDefault = false;
-      }
-    }
-  }

This only makes sense if we're dropping support for OS X 10.5, which
should be the subject of another bug.
(Assignee)

Comment 9

5 years ago
Created attachment 641902 [details] [diff] [review]
fix v1.4

Restored/fixed some comments, restored the printing code. We are planning to drop support for Mac OS X 10.5 in Firefox 17, this patch won't land until that is done.
Attachment #641099 - Attachment is obsolete: true
Attachment #641099 - Flags: review?(smichaud)
Attachment #641902 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #641902 - Flags: review? → review?(smichaud)
Attachment #641902 - Flags: review?(smichaud) → review+
(Assignee)

Comment 10

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=bd8caeb567c6
(Assignee)

Comment 11

5 years ago
Created attachment 644025 [details] [diff] [review]
fix v1.5

Fixes a compile error and removes some dead code.
Attachment #641902 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=dc37d03ced00
(Assignee)

Comment 13

5 years ago
Created attachment 656028 [details] [diff] [review]
fix v1.6

We're all set to land a patch for this now. This update syncs to latest trunk.
Attachment #644025 - Attachment is obsolete: true
Attachment #656028 - Flags: review?(smichaud)
(Assignee)

Comment 14

5 years ago
fix v1.6 try run:

https://tbpl.mozilla.org/?tree=Try&rev=d12b2a02432e
(Assignee)

Comment 15

5 years ago
Created attachment 656174 [details] [diff] [review]
fix v1.7

Compile fix, and remove more dead code.
Attachment #656028 - Attachment is obsolete: true
Attachment #656028 - Flags: review?(smichaud)
Attachment #656174 - Flags: review?(smichaud)
(Assignee)

Comment 16

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=7441e793aa77
Comment on attachment 656174 [details] [diff] [review]
fix v1.7

Looks fine to me.
Attachment #656174 - Flags: review?(smichaud) → review+
(Assignee)

Comment 18

5 years ago
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/c38da7c88d55
https://hg.mozilla.org/mozilla-central/rev/c38da7c88d55
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Assignee)

Updated

5 years ago
Blocks: 787510
Depends on: 788436

Updated

5 years ago
Depends on: 828216
Depends on: 829710
You need to log in before you can comment on or make changes to this bug.