661082 - cfx xpi is ignoring package.json's "url" property

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: Documentation, defect, P2)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks like rdf.py is checking for a "homepage" property, and then deletes the em:homepageURL element from install.rdf: 
if target_cfg.get("homepage"):
        manifest.set("em:homepageURL", target_cfg.get("homepage"))
    else:
        manifest.remove("em:homepageURL")

The package specification page says that the property is "url", not "homepage": https://jetpack.mozillalabs.com/sdk/1.0b5/docs/dev-guide/addon-development/package-spec.html (unless that has changed since 1.0b5)


Changing "homepage" to "url" makes it so the rdf element is not deleted.

Actual results: 
Documentation says to use "url", but the code checks for "homepage".

Expected results:
Either change the documentation to match the code or vice versa.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Brian: do you have any idea whether this should be a code or a docs change?  It isn't clear at first glance what our intention was here.

Comment 2

[Brian Warner [:warner :bwarner]]

Hm. I see four things that ought to agree on a single term:

 * package-spec.md documents "url"
 * packaging.py copies "url" into the metadata inside harness-options.json
 * rdf.py copies "homepage" into install.rdf's em:homepageURL field
 * http://wiki.commonjs.org/wiki/Packages/1.1 says "homepage", not "url"

I have no strong feelings one way or the other, so it feels to me like we
might as well match the CommonJS terminology.

That means changing the docs to say "homepage", and change packaging.py to
copy "homepage" instead of "url" into the metadata field. I'll write up a
patch now.

Comment 3

[Brian Warner [:warner :bwarner]]

Attachment: use "homepage", not "url"

KWierso: can you give this patch a try and see if it work for you?

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to comment #3)
> Created attachment 536762 [details] [diff] [review] [review]
> use "homepage", not "url"
> 
> KWierso: can you give this patch a try and see if it work for you?

With that patch, "homepage" in package.json is carried over to <em:homepageURL> in install.rdf.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 536762 [details] [diff] [review]
use "homepage", not "url"

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

Awesome, thanks warner!

Comment 6

[Brian Warner [:warner :bwarner]]

ok, I think the only thing keeping this from landing is the freeze, since this is scheduled for 1.0 but we've already made an RC.

myk: am I getting that right?

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Low risk, a=myk for commission during freeze.

Comment 8

[Brian Warner [:warner :bwarner]]

landed, in https://github.com/mozilla/addon-sdk/commit/7b7753bb891ea83b45a3f209eddf6970b801c147

myk: you'll want to cherry-pick that to the 1.0 branch