869223 - promise.js doesn't work with #ifdef

Status: RESOLVED INVALID

(Add-on SDK Graveyard :: General, defect)

Description

[Raymond Lee [:raymondlee]]

I've encounter an issue with importing promise.js and using #ifdef

If promise.jsm is imported into a js file and that js file contains statements e.g.  #ifdef XP_MACOSX to include code for particular platform, the following errors would occur.


Timestamp: 7/5/13 6:36:37 AM
Error: The character encoding of the plain text document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the file needs to be declared in the transfer protocol or file needs to use a byte order mark as an encoding signature.
Source File: resource://gre/modules/commonjs/sdk/core/promise.js
Line: 0

Timestamp: 7/5/13 6:36:43 AM
Error: SyntaxError: illegal character
Source File: resource://gre/modules/PlacesBackups.jsm
Line: 48


It's easy to reproduce this:
1) go to any js/jsm file which imports resource://gre/modules/commonjs/sdk/core/promise.js
2) add #ifdef XP_MACOSX and #endif
3) re-build that part of code and it would throw those errors.

Comment 1

[Dave Townsend [:mossop]]

Doubt this is anything to do with promise.js. More likely you aren't correctly pre-processing your js/jsm file. Can you give an example in-tree where this happens, or attach a patch that demonstrates the build failure?

Comment 2

[Raymond Lee [:raymondlee]]

Attachment: demo

This patch demonstrates the issue.

Before you apply the patch, go Bookmarks menu > Show All Bookmarks > Import and backup your bookmarks button > Restore, and you should see a bunch of backups.

After you apply the patch and rebuild the relevant source, do the same as above and you won't see those backups listed and there are errors on the console as I previously mentioned.

Please let me know whether you get the same errors.

Comment 3

[Dave Townsend [:mossop]]

Right, PlacesBackups.jsm isn't pre-processed. You need to move it in the Makefile.in into the EXTRA_PP_JS_MODULES section.

Comment 4

[Raymond Lee [:raymondlee]]

(In reply to Dave Townsend (:Mossop) from comment #3)
> Right, PlacesBackups.jsm isn't pre-processed. You need to move it in the
> Makefile.in into the EXTRA_PP_JS_MODULES section.

Thanks for your advice.  I didn't know about EXTRA_PP_JS_MODULES.