386537 - Plugins are incorrectly told that windowless support is available when it is not on X11

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Karl Tomlinson (:karlt)]

The documented method for finding out whether windowless support is available
is to use NPN_Version and NPVERS_HAS_WINDOWLESS:

http://developer.mozilla.org/en/docs/Gecko_Plugin_API_Reference:Version%2C_UI%2C_and_Status_Information#Finding_Out_if_a_Feature_Exists

However, since 1998-03-27, NPVERS_HAS_WINDOWLESS and NP_VERSION_MINOR have
indicated that the browser has windowless support for all platforms.

Furthermore, when the plugin sets NPPVpluginWindowBool to FALSE, NPN_SetValue
returns NPERR_NO_ERROR.  The plugin has no chance to know that anything is
wrong until is receives a NPP_Destroy call without having received any
NPP_SetWindow.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Set NPVERS_HAS_WINDOWLESS appropriately according to platform

This is ugly but it would work.

NP_VERSION_MINOR became 18 in March:

http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/modules/plugin/base/public/npapi.h&rev=3.44

Alpha releases since 1.9a4 will still report incorrectly but this would be
resolved in beta1 by addition of windowless support.

I would be concerned about changing an API that has been defined for almost a
decade and is used by other browsers but I doubt any other browsers offer
windowless support on X11.

If we don't do this, then plugins will probably end up checking
NP_VERSION_MINOR against 18 anyway unless they feel like parsing the
Gecko version from NPN_UserAgent.

Johnny, what do you think?

Comment 2

[Kaz Nishimura]

(In reply to comment #1)
> Created an attachment (id=270531) [details]
> Set NPVERS_HAS_WINDOWLESS appropriately according to platform
> 
> This is ugly but it would work.

I do not think this is the right fix.  The macro should be defined to tell if window-less mode is supported in that version of the API, not if it is properly implemented in that version of the browser.

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> The macro should be defined to tell if window-less mode is supported
> in that version of the API, not if it is properly
> implemented in that version of the browser.

You are right.

And another browser may wish to implement windowless support before
XPCONNECT_SCRIPTING, for example.

The bug is in the browser not the API, and it is pretty hard to fix bugs in
versions of the browser that have already been released.

The only thing that the plugin can do AFAIK is to use NPN_UserAgent to detect
the bugs in browsers that claim to have support but don't.

Marking WONTFIX because I don't think this can be fixed.

Comment 4

[Karl Tomlinson (:karlt)]

We could do a NPN_GetValue variable like NPNVSupportsXEmbedBool.

Comment 5

[Benjamin Otte]

We had a discussion about how to best solve this issue and my requirement as a plugin writer is pretty simple: Give me a method to detect reliably if aplugin implementation supports windowless on X11. This has to work for all released plugin host implementations (all Mozilla browsers and Konqueror, Opera and whatever else) as well as for all future releases.

Since Mozilla tainted the NPVERS_HAS_WINDOWLESS method, another method is required. User agent sniffing is probably very suboptimal, or can someone give me a C code snippet that correctly blacklists all previous browsers?

So I'm all for adding a new NPN_GetValue variable as it makes it much less pain for plugin and plugin host writers.

Comment 6

[Karl Tomlinson (:karlt)]

I checked out nsplugins from kdebase-3.5.7, and opera 9.23.  Although their NPN_Version functions return >= NPVERS_HAS_WINDOWLESS (0.11 and 0.16 respectively), at least neither of them returns NPERR_NO_ERROR from NPN_SetValue with NPPVpluginWindowBool.  That bug seems to only be in Gecko.  So user agent sniffing would only need to blacklist Gecko browsers.

However a big problem with user agent sniffing is that the user agent string can be changed by the user.  (When plugins use this they can end up with problems like bug 

As no other browser has yet provided windowless plugin support on X11, we still have the opportunity to require the API to provide a NPNV_SupportsWindowless variable even though that functionality seems redundant to the API.

Comment 7

[Karl Tomlinson (:karlt)]

...problems like bug 83376.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: NPNVSupportsWindowless

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in