707993 - xulrunner npapi.h tries to check value of MOZ_PLATFORM_MAEMO even if it is undefined

Status: RESOLVED DUPLICATE of bug 748466

(Core Graveyard :: Plug-ins, defect)

Description

[Tom "spot" Callaway]

Attachment: xulrunner-8.0-fix-maemo-checks-in-npapi.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111108090206

Steps to reproduce:

In the xulrunner source, the file dom/plugins/base/npapi.h contains a conditionalization check for MOZ_PLATFORM_MAEMO, but does not first check that MOZ_PLATFORM_MAEMO is defined (which it isn't, unless you're on MAEMO). The attached patch fixes this issue. This patch was taken from this commit:
http://code.google.com/p/npapi-sdk/source/detail?r=14

Comment 1

[Takanori MATSUURA]

(In reply to Tom "spot" Callaway from comment #0)
> Created attachment 579365 [details] [diff] [review]
> xulrunner-8.0-fix-maemo-checks-in-npapi.patch

Is this ready for review?

Comment 2

[Tom "spot" Callaway]

(In reply to Takanori MATSUURA from comment #1)
> (In reply to Tom "spot" Callaway from comment #0)
> > Created attachment 579365 [details] [diff] [review]
> > xulrunner-8.0-fix-maemo-checks-in-npapi.patch
> 
> Is this ready for review?

I'm not sure if you're asking me that question or not. It works for me in testing.

Comment 3

[Takanori MATSUURA]

(In reply to Tom "spot" Callaway from comment #2)
> I'm not sure if you're asking me that question or not. It works for me in
> testing.

So you should set "review?" flag and the reviewer to attachment 579365 [details] [diff] [review]. :-)

Comment 4

[Takanori MATSUURA]

Comment on attachment 579365 [details] [diff] [review]
xulrunner-8.0-fix-maemo-checks-in-npapi.patch

For your patch, Josh is the right reviewer.

https://wiki.mozilla.org/Modules/Core#Plugins

Comment 5

[Tom "spot" Callaway]

Thanks. I have no idea how I was supposed to know any of that. :) Perhaps bugzilla should be prompting people somehow?

Comment 6

[Josh Aas]

Comment on attachment 579365 [details] [diff] [review]
xulrunner-8.0-fix-maemo-checks-in-npapi.patch

Review of attachment 579365 [details] [diff] [review]:
-----------------------------------------------------------------

Changes to the NPAPI headers must be made to the upstream NPAPI header repository at:

http://code.google.com/p/npapi-sdk/

Once made there, we will sync with npapi-sdk.

Comment 7

[Josh Aas]

Sorry, I just noticed that this patch came from upstream. What we need, in that case, is just a bug requesting that we sync with the current revision of npapi-sdk.

Comment 8

[Tom "spot" Callaway]

I thought that's what this was. Do you need me to say "I request that you sync with the current revision of npapi-sdk"? :P

Comment 9

[Josh Aas]

Have you verified that our headers match the current state of npapi-sdk with your patch? In other words, is this the only difference between our headers and current npapi-sdk, or is there more?

Comment 10

[Tom "spot" Callaway]

The only change since your last commit to npapi-sdk (r13 "Add support for NPNVdocumentOrigin. r=smorgan") is this change (r14 "Re-add the lost defined() checks for MOZ_PLATFORM_MAEMO to prevent warnings").

However, the change in r13 does not seem to be applied in the xulrunner 8.0 bundled copy of npapi.h. So, a sync to npapi-sdk current would only apply those two changes, the one I posted above as a patch and your commit (r13).

Comment 11

[Tom "spot" Callaway]

Do you need me to submit a patch that accounts for r13 and r14 from upstream npapi-sdk? If not, can this be simply synced?