Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Currently, there is no way to use this module as a dependency in a profile. Working with OpenY we faced to a problem that when libraries folder is locating in a profile this module does not work, because of the path to the library is using only libraries folder in the root directory and module does not expect that libraries could be located in other places.
Proposed resolution
Add supporting module libraries in a case when this module installed, to be sure module will find needed library even if it is locating in a profile.
Remaining tasks
Review patch.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | colorbutton-module_can_t_find-2881822-15.patch | 601 bytes | SylvainM |
| #9 | module_can_t_find-2881822-9.patch | 615 bytes | finne |
| #5 | colorbutton-libraries-2881822-5.patch | 1.55 KB | ddrozdik |
Comments
Comment #2
ddrozdik CreditAttribution: ddrozdik as a volunteer and at FFW for Open Y commentedHere is a patch.
Comment #3
ddrozdik CreditAttribution: ddrozdik as a volunteer and at FFW for Open Y commentedHere is updated version of the patch.
Comment #4
hamrant CreditAttribution: hamrant at DEWEB Studio, Drupal Ukraine Community, FFW, Open Y for Drupal Ukraine Community commentedLooks good for me
Comment #5
ddrozdik CreditAttribution: ddrozdik as a volunteer and at FFW for Open Y commentedI found an issue with the latest patсh. Here is update version.
Comment #7
ddrozdik CreditAttribution: ddrozdik as a volunteer and at FFW for Open Y commentedComment #8
finneThis fix breaks the loading of the js file when you deploy to a subdir of the server document root (when the base path is not just "/"). It hard-codes the base path as "/".
Comment #9
finneAdded fixed patch that uses base_path() instead of "/".
Comment #10
finneComment #11
ddrozdik CreditAttribution: ddrozdik as a volunteer and at FFW for Open Y commentedComment #12
kevinquillen CreditAttribution: kevinquillen at Velir commentedThis was previously removed because it was breaking for others who had different configurations, like subdirectory installs of Drupal.
As much as I don't want to drag in the dependency, it seems unavoidable now to use the Libraries API so that all cases can be supported (where people are storing their libraries).
I've hesitated because it has no beta/stable release, but issues like this continue popping up on all CKEditor based plugins.
Comment #13
finneAre we talking about the same thing here?
Unpatched the path would be a relative path starting with "libraries/...".
Patch #5 adds an optional check on the Libraries module, but defaults to a hard coded path starting with "/libraries/...".
Patch #9 adds base_path() instead of "/" to the default option.
This way you don't depend on the Libraries module, but you can use it if it's present.
(yes a universal Libraries based solution would be ideal but as kevinquillen says it's not stable yet)
Comment #14
kevinquillen CreditAttribution: kevinquillen at Velir commentedYeah. I believe I had base_path in at one point, and removed it (could have been another CKEditor module) because it was not satisfying everyone. I think eventually we will need Libraries API to cover every possible use case, and have one common function for obtaining its location instead of assuming. But we need a somewhat stable release, first..
Comment #15
SylvainM CreditAttribution: SylvainM at Axess Open Web Services commentedCKEditorPluginInterface::getFile() doc says: "Returns the Drupal root-relative file path to the plugin JavaScript file."
So, when adding base_path() or "/", it becomes an absolute, and wrong, path: "/libraries/colorbutton"
I think it should be "libraries/colorbutton".
README says: "Place the plugin in the root libraries folder (/libraries).", so you can't place the library elsewhere.
attached patch
Comment #16
holist CreditAttribution: holist at Siili Solutions commentedIt seems to me also that @SylvainM is correct; using "/" in the beginning gives file system root, not Drupal root.
Comment #17
holist CreditAttribution: holist at Siili Solutions commentedLatest patch applies fine and does what it is supposed, I propose RTBC.
Comment #18
XTazWarning : file_get_contents(/libraries/colorbutton/plugin.js): failed to open stream
#15 works fine for me. Thanks
Comment #19
rooby CreditAttribution: rooby commentedNote that there seems to be some roadblocks with the libraries module, so I would +1 kevinquillen in not using that just yet. See #1704734: [master] Libraries API 8.x-3.x
Comment #20
Krzysztof Domański#15 was commited in #2919123: Warning: file_get_contents(/libraries/colorbutton/plugin.js).