Must show actual license (licence) content

VERIFIED FIXED in 5.0.7

Status

P2
normal
VERIFIED FIXED
10 years ago
3 years ago

People

(Reporter: davemgarrett, Assigned: davedash)

Tracking

unspecified
5.0.7

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

10 years ago
It should not just have a drop down list of licenses and a link for more info. The link isn't even on the manage version page, just the initial agreement. If developers are to agree to something they must have it there to agree to. The content of the selected license needs to be shown below. Developers should not be required to agree to a vague idea of a license, to be listed elsewhere. By not actually showing the licenses you are guaranteed to have developers who aren't in any way experts with licenses "agree" to things they simply did not agree to.

The short licenses need to have their entire content shown below the menu upon selection. The longer licenses could have just their preamble and/or a summary and a direct link to the official full version.
We should do the following:

Create htmlized versions of all the agreements with a landing page that has a link to a copy of each agreement.  This is simply a list of the supported licenses.

By the license selection boxes, include a "View licenses" option that pops this into a new tab.
Target Milestone: --- → 5.0.7
We should add this 'view licenses' link on the manage versions page in addition to the developer agreement, linked to the section of the FAQ that has links to licenses.  I checked with legal and we should not host copies of the licenses but instead point to official sites that have the licenses
Priority: -- → P2
Severity: blocker → normal
Assignee: nobody → dd-mozilla
Assignee: dd-mozilla → dd
I don't have proper links for the non GNU/Mozilla licenses:


    'http://www.opensource.org/licenses/mit-license.php',
    'http://www.opensource.org/licenses/bsd-license.php'   

Is what I have for MIT and BSD... if anybody has something more official, I'm all ears.

I don't have time to finish this for the manage versions page as well.  Especially since I'm giving my peers zero time to review, so I'll check in the Dev agreement stuff - or submit the patch.  We can always defer this to the next release.
Posted patch v1 (obsolete) — Splinter Review
Here's the fix for the dev center.... but that's all I could get done, and there may be issues with it so check carefully.  First cake/amo change and whatnot.
Attachment #385306 - Flags: review?(clouserw)
Comment on attachment 385306 [details] [diff] [review]
v1

Thanks Dave.  It looks fine overall, I've got 3 comments:

- Rather than making a new array let's just make $licenses associative. e.g. (___('licenses_mit') => 'http...').  Since we're storing this in the database I wouldn't object to hardcoding the ids in there either, e.g. [1] => array('' => ''), rather than depending on PHP to generate them correctly every time.
- the radio buttons should default to the previous add-on version's license if one exists.
- when clicking "other" the textarea that appears should be above the "Agree and continue" button 

Since this will change the structure of that array you'll probably have to do everything at once.  Official code freeze is next tuesday.  If you can't get it done before then we'll defer the entire bug.  Let me know what your schedule is. Thanks.
Attachment #385306 - Flags: review?(clouserw) → review-
I thought about doing that - but I'm not sure where else this  licenses array is used... and the comments in bootstrap seemed to imply that there are some annoyances.

But I see what you mean, just create a simple license structure - that could work.

What else do I need to "grep" for $licenses, global $licenses, $context->License... anything else to find out where this guy is used?

Okay it's possible to get this done by next tuesday - let me know about finding ther references to that licenses array.
(In reply to comment #6)
> I thought about doing that - but I'm not sure where else this  licenses array
> is used... and the comments in bootstrap seemed to imply that there are some
> annoyances.

The comments just mean we're using the order in the database.  So, the first one in that list is id 0 in the database.  It makes me nervous to have PHP calculate that on the fly, so I'd rather it was hardcoded.

> What else do I need to "grep" for $licenses, global $licenses,
> $context->License... anything else to find out where this guy is used?
$licenses should be all.  It's a global so it needs to be declared "global $licenses" in order to be used anywhere.  I wouldn't mind it just being in the License model instead of global to be honest but I guess there are arguments both ways.
I'm working on this today in my "spare time" - My primary task for today is the Download Stats that needs to be up tomorrow for FF3.5
Posted patch v2 (obsolete) — Splinter Review
Fixed all your issues - coded - but wasn't able to verify that the previous license of a version, influences the choice of new version's license.

So if you don't mind verifying that for me, that'd be awesome.

Also note that "other" is selected by default due to the rule that at least one radio button needs to be enabled.
Attachment #385306 - Attachment is obsolete: true
Attachment #385865 - Flags: review?(clouserw)
Comment on attachment 385865 [details] [diff] [review]
v2

This is mixed up with Jeff's paypal patches?
Attachment #385865 - Flags: review?(clouserw) → review-
Comment on attachment 385865 [details] [diff] [review]
v2


>- * Contributor(s):
>- *   Jeff Balogh <jbalogh@mozilla.com> (Original Author)

Please don't delete all my code Dave. ;)
what the shit...

I blame git... and therefore jeff...

new patch forthcoming
Posted patch v3 (obsolete) — Splinter Review
git svn rebase so I wouldn't delete all of Jeff's code.
Attachment #385865 - Attachment is obsolete: true
Attachment #385923 - Flags: review?(clouserw)
Comment on attachment 385923 [details] [diff] [review]
v3

> Fixed all your issues - coded - but wasn't able to verify that the previous
> license of a version, influences the choice of new version's license.
It, apparently, doesn't.  I really thought it looked at the license of your last version and use that if it existed but I don't see anywhere where that would have happened.  Weird.

I'll r+ it with two tiny changes:

> Also note that "other" is selected by default due to the rule that at least one
> radio button needs to be enabled.
Why do you have to enable a radio button?  I don't think anything should be pre-selected.

+<p>Please Choose:</p>
Change that to use ___('devcp_uploader_please_choose')
Attachment #385923 - Flags: review?(clouserw) → review-
w3c HTML recommendation for radio buttons is one button is checked.  The browser expectation is to default to the first item if there is no check - although this is never followed the same cross-browser hence the recommendation.  You could twist my arm into breaking spec... the specs are sometimes stupid.

I agree with the string change, however.
Posted patch v4Splinter Review
Attachment #385923 - Attachment is obsolete: true
Attachment #386066 - Flags: review?(rdoherty)
Attachment #386066 - Flags: review?(rdoherty) → review?(clouserw)
Comment on attachment 386066 [details] [diff] [review]
v4

thanks
Attachment #386066 - Flags: review?(clouserw) → review+
awesomepants:~/Projects/Addons/code % git svn dcommit
Committing to https://svn.mozilla.org/addons/trunk ...
        M       site/app/config/bootstrap.php
        M       site/app/controllers/components/developers.php
        M       site/app/models/license.php
        M       site/app/views/developers/uploader.thtml
        M       site/app/views/elements/developers/license_picker.thtml
        M       site/app/webroot/css/developers.css
        M       site/app/webroot/css/remora.css
        M       site/app/webroot/js/developers.js
Committed r29165
        M       site/app/views/elements/amo2009/search.thtml
r29134 = 9d37c3264ad7950f45ce26b87e7ba77b40d9fb02 (trunk)
        M       site/app/locale/en_US/LC_MESSAGES/messages.po
        M       site/app/models/addon.php
        M       site/app/controllers/developers_controller.php
        M       site/app/controllers/addons_controller.php
        M       site/app/views/admin/config.thtml
        M       site/app/views/developers/addon_edit_contributions.thtml
        M       site/app/views/addons/display.thtml
        M       site/app/views/elements/amo2009/contribution.thtml
        A       site/app/views/elements/config_radio.thtml
r29150 = 0989c1416237f9446d93e7784fe1534c5761d2ab (trunk)
        A       site/app/config/migrations/29150-paypal-killswitch.sql
r29151 = 6cb353713748f39ef310f9f104c0f39d682adc67 (trunk)
        M       site/app/views/addons/display.thtml
r29158 = f7f23de2a29486593e501caefdd02888e922e050 (trunk)
        M       site/app/views/addons/display.thtml
r29159 = df8487d1deaac3cbe3cf07e3a078d481cd94439b (trunk)
        M       site/app/locale/en_US/LC_MESSAGES/messages.po
        M       site/app/webroot/css/amo2009/main.css
        M       site/app/models/addon.php
        M       site/app/models/user.php
        M       site/app/config/routes.php
        M       site/app/controllers/addons_controller.php
        A       site/app/views/addons/developers.thtml
r29160 = bad5c635c1812f4026f9e3df8fea8b5c8c0e9f7f (trunk)
        M       site/app/views/search/index.thtml
r29161 = 77968cf90fe39b8363d5823e08da69f24c9d5931 (trunk)
        M       site/app/views/search/index.thtml
r29162 = ff7307204a72ec2fae8ec2d2e1f59677aef6d91f (trunk)
        M       site/app/views/search/index.thtml
r29163 = aeec6d6872281b67a87e99340a1ec2225e70489e (trunk)
        M       site/app/views/elements/amo2009/search.thtml
r29164 = 10d1494e95c8a7d734b7f8f8867cd3e547833cf3 (trunk)
        M       site/app/webroot/css/developers.css
        M       site/app/webroot/css/remora.css
        M       site/app/webroot/js/developers.js
        M       site/app/models/license.php
        M       site/app/config/bootstrap.php
        M       site/app/controllers/components/developers.php
        M       site/app/views/developers/uploader.thtml
        M       site/app/views/elements/developers/license_picker.thtml
r29165 = f77a6361f4e591ef2c3f81936e51d7442bcd090d (trunk)
W: 642ce010bafc9307442571e5f97834d507e6a812 and refs/remotes/trunk differ, using rebase:
:040000 040000 e495331dbca3d11d9f06bb81ded887e7ab810399 29f297b6e5077d35d831b69b0e39e8261676bf37 M      site
First, rewinding head to replay your work on top of it...
Applying: [bug 491209] Polished up the radio button interface for license selection.
/Users/dash/Projects/Addons/code/.git/rebase-apply/patch:76: trailing whitespace.
        
/Users/dash/Projects/Addons/code/.git/rebase-apply/patch:83: trailing whitespace.
        
/Users/dash/Projects/Addons/code/.git/rebase-apply/patch:159: trailing whitespace.
    <?php foreach ($licenses as $val => $license): 
/Users/dash/Projects/Addons/code/.git/rebase-apply/patch:182: trailing whitespace.
#step-agreement .submit-area { text-align: right; } 
warning: 4 lines add whitespace errors.
Applying: [bug 4921209] Using gettext and ponies
        M       site/app/config/bootstrap.php
        M       site/app/controllers/components/developers.php
        M       site/app/models/license.php
        M       site/app/views/developers/uploader.thtml
        M       site/app/views/developers/versions_edit.thtml
        M       site/app/views/elements/developers/license_picker.thtml
        M       site/app/webroot/css/developers.css
Committed r29166
        M       site/app/webroot/css/developers.css
        M       site/app/models/license.php
        M       site/app/config/bootstrap.php
        M       site/app/controllers/components/developers.php
        M       site/app/views/developers/uploader.thtml
        M       site/app/views/developers/versions_edit.thtml
        M       site/app/views/elements/developers/license_picker.thtml
r29166 = 2734194dd78e5e2530776d24fd05fcab6bc3decc (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
site/app/controllers/components/developers.php: locally modified
site/app/views/elements/developers/license_picker.thtml: locally modified
        M       site/app/controllers/components/developers.php
        M       site/app/views/elements/developers/license_picker.thtml
Committed r29167
        M       site/app/controllers/components/developers.php
        M       site/app/views/elements/developers/license_picker.thtml
r29167 = 786db8af7d48c3afd585a849cb58300667a2f336 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Verified FIXED on https://preview.addons.mozilla.org/en-US/developers/versions/add/9331, with JavaScript both enabled and disabled.

Dave -- this was a great suggestion; thanks for filing the bug!
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.