User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_7) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.68 Safari/534.24 Build Identifier: If the extension name in package.json contains a period character ( eg 'fire.fox' ) cfx run crashes. Reproducible: Always Steps to Reproduce: 1. mkdir fire.fox && cd ./fire.fox 2. cfx init 3. cfx run Actual Results: Traceback (most recent call last): File "/Users/jeff/code/moz/jetpack/addon-sdk-1.0b5/bin/cfx", line 29, in <module> cuddlefish.run() File "/Users/jeff/code/moz/jetpack/addon-sdk-1.0b5/python-lib/cuddlefish/__init__.py", line 636, in run include_dep_tests=options.dep_tests File "/Users/jeff/code/moz/jetpack/addon-sdk-1.0b5/python-lib/cuddlefish/packaging.py", line 283, in generate_build_for_target add_section_to_build(target_cfg, "tests", is_code=True) File "/Users/jeff/code/moz/jetpack/addon-sdk-1.0b5/python-lib/cuddlefish/packaging.py", line 258, in add_section_to_build validate_resource_hostname(name) File "/Users/jeff/code/moz/jetpack/addon-sdk-1.0b5/python-lib/cuddlefish/packaging.py", line 82, in validate_resource_hostname raise ValueError('invalid resource hostname: %s' % name) ValueError: invalid resource hostname: jid1-gwb55aaeefl6yq-goo.gl_shortener-test Expected Results: Firefox starts up with the extension enabled.
This is either a docs bug or a pretty quick regex fix. Possible patches to follow.
Created attachment 533535 [details] [diff] [review] Patch to the docs stating the name field should not contain periods. Just patch the docs to clarify that the field in question cannot contain periods.
Created attachment 533536 [details] [diff] [review] Alternate patch that actually adds periods to the regex. Dashes and underscores are already there, periods seem innocuous but I'm unsure what larger spec governs these values in install.rdf. With this patch applied, a simple extension with a period in the name field seems to work correctly.
It's conceivable that a developer just starting with Jetpack could create a folder with a dot in it, follow the tutorial and then get an obscure error message when running the example extension - admittedly an edge case ;).
This sounds like a bug we already have filed, although I wasn't able to find it in a quick search. Irakli, Brian: do you know if this is a duplicate of another bug?
It's similar to this one, but not exactly the same: https://bugzilla.mozilla.org/show_bug.cgi?id=568131 I searched first! Really! I also did a quick look for a spec on what should or shouldn't be allowed in that field and didn't see any comments on whether periods are inherently bad - not an exhaustive search though, there was a hockey game on.
At this point, I'm in favor of adding docs and assertions rather than allowing more characters (and being surprised by something that doesn't like them). Addon names (and package names) turn into resource: URI fields that are treated like DNS names, so I wouldn't be surprised that periods cause trouble. So I'll go for the docs patch rather than the regex one (which, incidentally, appears to be reversed). If folks agree I'll land it tomorrow morning.
Assignee: nobody → warner-bugzilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
If you're going to dis-allow periods, it might be better to provide a better error message? I've got a new patch that a) reverses the last diff ( d'oh ) and b) detect periods and spaces in the same statement and return a better error message for that case.
Created attachment 533888 [details] [diff] [review] Patch to fix the docs ( as previous ) and improve error messages from packaging.py
ps This is all committed to my fork: https://github.com/canuckistani/addon-sdk Let me know if you want a pull request, or further changes.
Landed in https://github.com/mozilla/addon-sdk/commit/ad0498464ab774dba4b410460dc51ea85f94ff37 . I made a few small changes to fix one of the doctests (use "cfx testcfx" to run those). Thanks for the patch!
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Changing assignee to Jeff, as he contributed most of the patch, even though it was landed with some modifications by Brian.
Assignee: warner-bugzilla → jeff
OS: Mac OS X → All
Hardware: x86 → All
You need to log in before you can comment on or make changes to this bug.