Update jscolor to 2.5.1 #2937

luigifab · 2023-01-14T21:42:05Z

Description

This is an update of jscolor from v1.3.1 to v2.5.1.
GitHub repository: https://github.com/EastDesire/jscolor

The minified file is edited to keep compatibility with previous version: this.hash=false / jsc.pub.lookupClass="color"

Tested with Firefox 36/108, Chrome 32/108, Opera 19/94, Edge 108.
Tested from product view and configuration view.

before:

after:

Manual testing scenarios

In any system.xml, add in config/sections/anywhere/groups/anywhere/fields:

<color1>
	<label>Color</label>
	<validate>color</validate>
	<sort_order>1</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</color1>
<color2>
	<label>Color with option</label>
	<validate>color {hash:true}</validate>
	<sort_order>2</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</color2>

Don't forget to load the JS for jscolor in adminhtml layout:

<adminhtml_system_config_edit>
	<reference name="head">
		<action method="addItem">
			<type>js</type>
			<name>jscolor/jscolor.js</name>
		</action>
	</reference>
</adminhtml_system_config_edit>

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All automated tests passed successfully (all builds are green)
Add yourself to contributors list

ADDISON74 · 2023-01-14T21:56:23Z

Let's make a rule and keep unminified javascript/css files. This helps to check easy future modifications/issues.

justinbeaty · 2023-01-14T23:30:32Z

Let's make a rule and keep unminified javascript/css files. This helps to check easy future modifications/issues.

Agreed. I would suggest jscolor.js and jscolor.min.js. Old extensions may end up requiring the unminified version by doing this, but it's okay IMO.

sreichel

but it's okay IMO

I generally agree with @ADDISON74.

In that case, it's probably okay.

luigifab · 2023-01-15T09:51:33Z

Perhaps I can change the PR by adding unminified modified version and the minified version with a map file (and provide the command line used).

But if you have multiple devs, some of them can load unminified version, and other minified version. Perhaps I can add something like if ($name == 'jscolor/jscolor.js') $name = jscolor/jscolor.min.js; in function addItem. I don't know.

ADDISON74 · 2023-01-15T14:29:20Z

The most important thing is to be able to follow the changes, a comparison between files on two columns. If I am dealing with a minified file then I have to allocate time to make the file readable. Even if I do this, the PR remains in an unreadable form and I cannot show the others possible issues in the code. I think that the two variants and the map are more than enough.

I will test this PR because I am using extensions based on this JS file.

fballiano · 2023-01-15T14:55:14Z

they changed the license from LGPL to GPL3? I'm really not sure GPL3 is compatible with openmage's licenses, we have to be 100% sure about that before merging this.

justinbeaty · 2023-01-15T15:14:03Z

Good point regarding GPLv3 @fballiano. Worst case scenario, we can use 1.4.5.

ADDISON74 · 2023-01-15T15:29:18Z

If I look over the GPLv3 requirements (https://en.wikipedia.org/wiki/GNU_General_Public_License), the use of jscolor does not imply any issues, apart from the known ones that must be respected. Visiting the jscolor website I did not find anything about a fee per usage, it is obviously free. We could ask the developer's consent to use the latest versions in OpenMage.

luigifab · 2023-01-15T16:23:41Z

I will write a letter to Santa Claus...

justinbeaty · 2023-01-15T16:30:24Z

With the disclaimer that I'm not a lawyer, I think including the GPLv3 jscolor library is okay. IMO it can be considered a separate work for these reasons:

By default, we do not include or rely on jscolor anywhere in the code. Users must manually add the javascript file to the <head> element.
There is no Magento JS code that calls any function from jscolor. Instead, jscolor simply looks for DOM elements with class color and activates the color picker on them. It does this independently from all Mage JS code.
If jscolor was removed, nothing breaks. Instead users just have to enter a hex code into the input.

However, confirmation from the author would be nice.

ADDISON74 · 2023-01-15T16:37:19Z

These radical changes are damn challenging because we can't be sure if we have covered all the issues that may arise.

I tested this PR (jscolor version 2.5.1) with a test installation based on OM 19 updated today with git and the MageWorx Advanced Product Options extension, which uses jscolor shipped in Magento.

Here is the result in the current version. No error in the browser console.

Here is the result checking out this PR. Errors in the browser console when clicking on that small color swatch or in the input text box, where the color is filled in from the keyboard.

Here is the result with the last version in branch 1. The errors in the console are due to the fact that I did not add the image files used in the JS file. At first glance, the update to 1.4.5 did not raise any issues.

In conclusion, there could be major changes in version 2 that create issues in extensions like the one I teste and since there is no longer support for them, everyone will have to find a solution. This will generate dissatisfaction and we must avoid reproaches as much as possible. It is not an important component in OpenMage to be worth a major update effort. I would still update to version 1.4.5 and leave it in this shape from now on.

fballiano · 2023-01-15T17:00:54Z

I think it should really be avoided to GPL code in OM. GPL is not compatible with almost any license (https://gplv3.fsf.org/wiki/index.php/Compatible_licenses) and it is known for being a "viral" license:

For example, if a developer writes and distributes Program A that incorporates a third party’s Library B which is licensed under the GPL, Program A must also be licensed under the GPL. It should be noted that Program A does not automatically become licensed under the GPL just because it incorporates Library B, but it would breach the terms of Library B’s licence if Program A was distributed under a non-GPL licence.

taken from:
https://www.clendons.co.nz/resources/articles-and-publications/technology/using-gpl-code-your-software-essentials/

I've loved *GPL licenses since many years but OM is released under OSL and AFL and there's no way around it, so AFAIK we have to avoid including any GPL code. If it was required via composer it would maybe be better although it's debatable and just source of headaches.

justinbeaty · 2023-01-15T17:14:04Z

taken from: https://www.clendons.co.nz/resources/articles-and-publications/technology/using-gpl-code-your-software-essentials/

From the same article:

Proprietary programs may also be able to use GPL-licensed software without licensing issues if the proprietary program and the GPL-licensed software are, in substance, separate programs that operate at ‘arms-length’.

Which is what I meant by no code in OM calling any function in jscolor, so there is at least some opinion that they are separate programs. But, because we aren't lawyers, I agree it's probably easier to just not include GPL code. Plus, that it breaks extensions as observed by Addison.

I say to just use 1.4.5.

fballiano · 2023-01-15T17:21:12Z

GPL was never made for web softwares, that's why they specifically talk about static/dynamic linking. AGPL is much more clear about it (although restrictive).

The new model jscolor is using (https://jscolor.com/download/#licenses) may be the base for problems since OM extensions or stores (that use jscolor) could be considered (and actually are) commercial products in need of licensing.

Better to use 1.4.5 and probably find an alternative altogether.

justinbeaty · 2023-01-15T17:31:31Z

Perhaps it would be nice to support:

<frontend_type>color</frontend_type>

To generate an HTML5 element:

<input type="color">

But, that is for another time. We still can upgrade the existing jscolor to 1.4.5 for modules that require it.

ADDISON74 · 2023-01-15T18:01:03Z

In OM, as far as I know, there is no place where the color picker features is used. jscolor was added to the distribution just to be there in case a developer wants to use it. It's a relic from before HTML5. In the past there was a PR that proposed to remove this library from the distribution, but I asked to be kept because there are extensions that use it. Regarding the license, the problem arises between the developer of the extension and the author of the JS library.

I agree with an update to version 1.4.5, but only by replacing the file, without minified or map versions, exactly as provided by the developer. If it was important for OM and we was using it in the Backend, then I would have proposed removing it and installing it with Composer. As you may see, there are issues with unmaintained extension, license usage.

Was approved too hastily.

sreichel · 2023-01-16T00:40:05Z

In OM, as far as I know, there is no place where the color picker features is used. jscolor was added to the distribution just to be there in case a developer wants to use it. It's a relic from before HTML5.

I thought its just a version update ...

#2937 (comment)

Good point.

we cant upgrade to latest version
it is not used in core code
it is not required anymore (html5)

... delete it and add #2945?

ADDISON74 · 2023-01-16T01:20:47Z

We cannot delete it because it is used by 3rd party extensions. It was deleted once but we put it back based on my feedback.

An upgrade to 1.4.5 is just fine and forget about it.

sreichel · 2023-01-16T06:23:48Z

How about upgrade to 1.4.5 make make it optional (via composer)?

EastDesire · 2023-01-16T09:16:23Z

I'm giving you, the authors and contributors of OpenMage, my permission to use any version of jscolor in OpenMage and its derived projects/branches.

luigifab · 2023-01-16T09:23:30Z

@ADDISON74 For other modules, I'm sad for them, but in same time, PHP 9 will kill them.
Perhaps the problem you encounter is not complicated to solve, I don't know.
I also understand your point.

For input type color, yes I know it, I use it for a custom dev, but I removed it when I saw that jscolor is not abandoned.
Since I don't know when, jscolor support colors with alpha (#RRGGBBAA) and predefined colors.

sreichel · 2023-01-16T09:31:43Z

I'm giving you, the authors and contributors of OpenMage, my permission to use any version of jscolor in OpenMage and its derived projects/branches.

Great. Thank you!

@EastDesire @Flyingmana i think we need a png logo to add it to https://www.openmage.org/partners/index.html

fballiano · 2023-01-16T09:34:37Z

Thanks a lot @EastDesire.

I still think that, unless we have a special license (like LGPL) version we shouldn't have version 2, the problem is not really ours but of the final customer.

@sreichel is it possible to add a js library via composer?

sreichel · 2023-01-16T10:18:55Z

@sreichel is it possible to add a js library via composer?

At least via command/script. Until we add npm packages, we should add files to js/ (?)

ADDISON74 · 2023-01-16T10:31:05Z

@luigifab - Obviously it is not complicated to solve it, but what I presented is a popular extension in the market with many users. That it can be others. If we make the change to version 2 we will blow up the functionality. Any change in the source code must take into account the rules and target the comfortable user who does not walk around making changes as long as everything works normally. The popularity of a product is not determined by its producers, but by users. A shopping cart is popular when the user with average knowledge is able to modify it according to his needs.

Analyzing the situation better, I would update version 1.4.5 with the original name and add version 2 to the directory as jscolor2.js. The first one I prefer without Composer, we updated it and move forward.

fballiano · 2023-01-16T11:19:31Z

Since jscolor is not even in used by the core i would not add jscolor2.js and again we have to be extremely careful with the license, we cannot have GPL software inside our repo.

ADDISON74 · 2023-01-16T11:40:30Z

Indeed OSL is compatible with GPL. In this case, the current jscolor.js file which is version 1.3.1 must be replaced with version 1.4.5 and and there is nothing left to do.

I did not check if the image files in the directory were changed between versions.

fballiano · 2023-01-19T11:31:51Z

I will close this, if somebody wants to create a PR to upgrade to 1.3.1 that's welcome for sure.

Update jscolor to 2.5.1

56e6bd9

github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml JavaScript Relates to js/* Template : admin Relates to admin template labels Jan 14, 2023

sreichel previously approved these changes Jan 15, 2023

View reviewed changes

fballiano mentioned this pull request Jan 15, 2023

Added frontend_type color #2945

Merged

fballiano marked this pull request as draft January 16, 2023 12:45

fballiano closed this Jan 19, 2023

luigifab deleted the jscolor branch January 19, 2023 11:32

ADDISON74 added the Don't forget this PR label Jan 19, 2023

justinbeaty mentioned this pull request Jan 19, 2023

Update jscolor to 1.4.5 #2976

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update jscolor to 2.5.1 #2937

Update jscolor to 2.5.1 #2937

luigifab commented Jan 14, 2023 •

edited

Loading

ADDISON74 commented Jan 14, 2023

justinbeaty commented Jan 14, 2023

sreichel left a comment •

edited

Loading

luigifab commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

luigifab commented Jan 15, 2023

justinbeaty commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023 •

edited

Loading

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023 •

edited

Loading

ADDISON74 commented Jan 15, 2023

sreichel commented Jan 16, 2023

ADDISON74 commented Jan 16, 2023 •

edited

Loading

sreichel commented Jan 16, 2023

EastDesire commented Jan 16, 2023

luigifab commented Jan 16, 2023

sreichel commented Jan 16, 2023

fballiano commented Jan 16, 2023

sreichel commented Jan 16, 2023

ADDISON74 commented Jan 16, 2023

fballiano commented Jan 16, 2023 •

edited

Loading

ADDISON74 commented Jan 16, 2023

fballiano commented Jan 19, 2023

Update jscolor to 2.5.1 #2937

Update jscolor to 2.5.1 #2937

Conversation

luigifab commented Jan 14, 2023 • edited Loading

Description

Manual testing scenarios

Contribution checklist

ADDISON74 commented Jan 14, 2023

justinbeaty commented Jan 14, 2023

sreichel left a comment • edited Loading

Choose a reason for hiding this comment

luigifab commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

luigifab commented Jan 15, 2023

justinbeaty commented Jan 15, 2023

ADDISON74 commented Jan 15, 2023

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023 • edited Loading

fballiano commented Jan 15, 2023

justinbeaty commented Jan 15, 2023 • edited Loading

ADDISON74 commented Jan 15, 2023

sreichel commented Jan 16, 2023

ADDISON74 commented Jan 16, 2023 • edited Loading

sreichel commented Jan 16, 2023

EastDesire commented Jan 16, 2023

luigifab commented Jan 16, 2023

sreichel commented Jan 16, 2023

fballiano commented Jan 16, 2023

sreichel commented Jan 16, 2023

ADDISON74 commented Jan 16, 2023

fballiano commented Jan 16, 2023 • edited Loading

ADDISON74 commented Jan 16, 2023

fballiano commented Jan 19, 2023

luigifab commented Jan 14, 2023 •

edited

Loading

sreichel left a comment •

edited

Loading

justinbeaty commented Jan 15, 2023 •

edited

Loading

justinbeaty commented Jan 15, 2023 •

edited

Loading

ADDISON74 commented Jan 16, 2023 •

edited

Loading

fballiano commented Jan 16, 2023 •

edited

Loading