GNOME Bugzilla – Bug 563826
Small modifications to totem mozilla plugin to enable it to work in OpenOffice.org
Last modified: 2012-04-21 23:19:18 UTC
Please describe the problem: OpenOffice.org 3.0.1 will have the feature to embed mozilla plugins into presentations and documents working again. The target plugin is the adobe flash plugin. I'd like there to be a working free a/v mozilla-targeted plugin that can be used in OOo. The attached patch makes this happen, i.e. a) Don't consider not getting a mBaseURI fatal b) A height of -1 is still "max" height as the comment somewhere else in that .c says to avoid getting --audio-only tagged on c) Allow a smaller mozilla vtable than the one we build against if its big enough to support what we use Steps to reproduce: oowriter->Insert->Object->Video-> And use a .ogg would be nice to have working in openoffice.org-core-3.0.1 Actual results: Expected results: Does this happen every time? Other information:
Created attachment 124253 [details] [review] patch to do that
I'll leave it to Bastien to judge if this is worth it or not; meanwhile here are a few comments on the patch: + bool bOK = true; Totem is not OOo :) + // Offset of highest used function we can't live without + if (aMozillaVTable->size < offsetof(NPNetscapeFuncs, forceredraw) + sizeof(void*)) This is wrong. We ues the NPRuntime APIs, so we need at least up to and including |setexception| , corresponding to NP_VERSION_MINOR == 14, iirc. So you should check for this version, *and* the vtable size >= offsetof (NPNetscapeFuncs, pushpopupsenabledstate) which is next after setexception. Or check for version 13, and conditionalise the scripting on version >= 14.
Comment on attachment 124253 [details] [review] patch to do that >diff -ru totem-2.24.3.orig/gstreamer/browser-plugin/totemPlugin.cpp totem-2.24.3/gstreamer/browser-plugin/totemPlugin.cpp >--- totem-2.24.3.orig/gstreamer/browser-plugin/totemPlugin.cpp 2008-10-08 17:38:55.000000000 +0100 >+++ totem-2.24.3/gstreamer/browser-plugin/totemPlugin.cpp 2008-12-05 16:39:04.000000000 +0000 >@@ -1865,16 +1865,18 @@ > * error code from the NewStream function. > */ > >+ bool bOK = true; What does bOK mean? Can you find a more descriptive variable name please. > NPError err; > err = NPN_GetValue (mNPP, > NPNVPluginElementNPObject, > getter_Retains (mPluginElement)); > if (err != NPERR_NO_ERROR || mPluginElement.IsNull ()) { > D ("Failed to get our DOM Element NPObject"); >- return NPERR_GENERIC_ERROR; >+ bOK = false; > } > > #if defined(TOTEM_COMPLEX_PLUGIN) && defined(HAVE_NSTARRAY_H) >+ if (bOK) { > /* FIXMEchpe untested; test this! */ > if (!NPN_GetProperty (mNPP, > mPluginElement, >@@ -1882,10 +1884,12 @@ > getter_Retains (mPluginOwnerDocument)) || > mPluginOwnerDocument.IsNull ()) { > D ("Failed to get the plugin element's ownerDocument"); >- return NPERR_GENERIC_ERROR; >- } >+ bOK = false; >+ } >+ } You're changing a tab to spaces for no reasons. > #endif /* TOTEM_COMPLEX_PLUGIN */ > >+ if (bOK) { > /* We'd like to get the base URI of our DOM element as a nsIURI, > * but there's no frozen method to do so (nsIContent/nsINode isn't available > * for non-MOZILLA_INTERNAL_API code). nsIDOM3Node isn't frozen either, >@@ -1900,11 +1904,11 @@ > getter_Copies (baseURI)) || > !baseURI.IsString ()) { > D ("Failed to get the base URI"); >- return NPERR_GENERIC_ERROR; > } > > mBaseURI = g_strdup (baseURI.GetString ()); >- D ("Base URI is '%s'", mBaseURI ? mBaseURI : ""); >+ D ("Base URI is '%s'", mBaseURI ? mBaseURI : ""); >+ } > > /* Setup DBus connection handling */ > GError *error = NULL; >@@ -2178,7 +2182,7 @@ > } > #endif /* TOTEM_GMP_PLUGIN */ > #if defined(TOTEM_NARROWSPACE_PLUGIN) || defined (TOTEM_BASIC_PLUGIN) >- if (height <= 16 && !mControllerHidden) { >+ if (height != -1 && height <= 16 && !mControllerHidden) { > mAudioOnly = true; > } > #endif /* TOTEM_NARROWSPACE_PLUGIN || TOTEM_BASIC_PLUGIN */ >diff -ru totem-2.24.3.orig/gstreamer/browser-plugin/totemPluginGlue.cpp totem-2.24.3/gstreamer/browser-plugin/totemPluginGlue.cpp >--- totem-2.24.3.orig/gstreamer/browser-plugin/totemPluginGlue.cpp 2008-09-01 11:32:53.000000000 +0100 >+++ totem-2.24.3/gstreamer/browser-plugin/totemPluginGlue.cpp 2008-12-08 12:32:57.000000000 +0000 >@@ -405,17 +405,20 @@ > if ((aMozillaVTable->version >> 8) > NP_VERSION_MAJOR) > return NPERR_INCOMPATIBLE_VERSION_ERROR; > >- if (aMozillaVTable->size < sizeof (NPNetscapeFuncs)) >+ // Offset of highest used function we can't live without C-style comments please >+ if (aMozillaVTable->size < offsetof(NPNetscapeFuncs, forceredraw) + sizeof(void*)) > return NPERR_INVALID_FUNCTABLE_ERROR; >+ > if (aPluginVTable->size < sizeof (NPPluginFuncs)) > return NPERR_INVALID_FUNCTABLE_ERROR; > >- /* Copy the function table. We can use memcpy here since we've already >- * established that the aMozillaVTable is at least as big as the compile- >- * time NPNetscapeFuncs. >- */ >- memcpy (&NPNFuncs, aMozillaVTable, sizeof (NPNetscapeFuncs)); >- NPNFuncs.size = sizeof (NPNetscapeFuncs); >+ /* Copy the function table. >+ */ Comment is one line, so please close it on the same line. Please add a better comment about what follows. >+ uint16 size = aMozillaVTable->size < sizeof (NPNetscapeFuncs) ? >+ aMozillaVTable->size : sizeof (NPNetscapeFuncs); Please use an if conditional rather than a ternary here, it's quite difficult to read otherwise (especially when not indented properly). >+ memset (&NPNFuncs, 0, sizeof (NPNetscapeFuncs)); >+ memcpy (&NPNFuncs, aMozillaVTable, size); >+ NPNFuncs.size = size; > #if 0 // FIXMEchpe > /* Do we support XEMBED? */ > NPError err; Functionally-wise, looks good.
Caolan, any update ?
Nah, I couldn't get past the mental block of C++ comments disallowed in C++ code :-). Might just take it up from the other side and extend the OOo coverage of the plugin api
I'm guessing this isn't relevant anymore and LibreOffice's plugin support got enhanced. Reopen if you have an updated patch at some point.