jWYSIWYG 0.92 detection is broken (they was changed version string). Attached patch.
Also download location was changed - now latest editor available at http://github.com/akzhan/jwysiwyg/ (this is indicated at http://code.google.com/p/jwysiwyg/source/browse/trunk/README.txt)
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | 923460-18.patch | 1.18 KB | rpayanm |
Comments
Comment #1
twodThe only real change here is that the $ has been removed. We're only interested in the version number, optionally matching
plugin\sdoes not help locating that. Removing the trailing $ only has the effect that we'd now risk matching another version number which is not at the end of the line (say they decide to include which version of jQuery they depend on in the same line).The text part of the version line in jWysiwyg hasn't changed at all since at least as early as 0.8 (didn't check further back), and this regular expression has been working perfectly, so why make these irrelevant changes now?
In fact, Wysiwyg is perfectly able to detect all versions of jWysiwyg, up to and including 0.93, which I just tested with.
You will however get a "version could not be detected" error if you extract everything in the jWysiwyg package to sites/all/libraries/jwysiwyg, as starting with version 0.6 they include the jQuery library too. jWysiwyg itself has been moved to a "jwysiwyg" subfolder. The installation instructions on admin/settings/wysiwyg specifically mention that jquery.wysiwyg.js must be at sites/[all|sitename]/libraries/jwysiwyg/jquery.jwysiwyg.js, meaning you should not upload the jQuery library bundled with jWysiwyg - as Drupal already provides this - just the jWysiwyg library itself found in the "jwysiwyg" subfolder.
Yes, it's a bit confusing, since we don't officially support modification of editor libraries it might also sound like a contradiction. If we consider these as two separate libraries which have just been packed together, we can get away with this "modification" by saying we're not actually modifying either library, just cherry picking parts of the bundle to get just the library we want. We still don't support modifications inside that library.
---
Now to something which will actually have to change, but was not part of the patch: The URLs....
I don't have my development server running at the moment so I can't create a patch for this, will just rename and leave the issue as "needs work" for now.
Powered by Dreditor.
Comment #2
ph0enix commentedI have extracted by instruction, so path to editor was sites/all/libraries/jwysiwyg/jquery.jwysiwyg.js
jwysiwyg 0.92 has following version string:
So after version not EOL. I have added 'plugin' string check to make sure that we are selecting version - not another float number. I know this not a clean.
In 0.93 version number finished by EOL, so patch is not needed
Changelog:
Comment #3
twodOh, sorry I missed that, my bad. Must have tried 0.93 twice istead of 0.92.
Your changes are pretty sound then, but can we change the first parentheses so it doesn't "remember" that match for backreferences? (If I remember correctly, this is done the same way in PHP and JavaScript, by doing
(?:plugin\s).) Without the EOL we might need that. If the 'plugin' string is in all earlier versions it might be a good idea to make it required too, and pehaps expand the pattern to target the whole editor name (case insensitive match).That way the last change isn't needed as only one match is stored.
Comment #4
ph0enix commentedYes, regexp
@(?:plugin\s+)([0-9\.]+)@is working and "plugin" match is not saved. Tested on 0.6, 0.92 and 0.93.I have just checked version changes from 0.1 to 0.93. String "WYSIWYG - jQuery plugin <version>" is presented in all releases, so it possible to include "plugin" into required match.
Comment #5
twodOk, this patch should do the trick.
Updated the URLs and changed the Regular Expression as discussed above, just removed the first parenthesis as that part is not optional now anyway.
Made a few quick tests and this recognizes the version strings fine, including 0.92. URLs took me to the right pages as well.
Small change so I'll commit this right away after first confirmation. =)
Comment #6
twodVery small change and easy to test, so bumping this to RTBC to give it some more attention.
Comment #7
sunWanted to commit this, but then recognized that the download package generated by github is also different. I'm 100% sure this will circle back into support requests.
Perhaps we should contact the author to provide some cleaner/better download packages.
Comment #8
sunComment #9
sunCreated an issue upstream: https://github.com/akzhan/jwysiwyg/issues/issue/42
Comment #10
sunNo response over there yet, so this will have to wait a bit.
Comment #11
frost-nzcr4 commentedhttps://github.com/akzhan/jwysiwyg/issues/#issue/42/comment/634076
Comment #12
sunReading up on this issue once again, it seems like only one version (0.92) could not be detected, but that seems to have been fixed upstream.
What remains is the new website and the new release package structure. Regarding the latter, I still hope that the jwysiwyg project is able to restore a more simple (and backwards-compatible) structure, since supporting the two entirely different file locations is going to be a very complex task for us.
Comment #13
frost-nzcr4 commentedhttps://github.com/frost-nzcr4/jwysiwyg/tree/drupal
Comment #14
frost-nzcr4 commentedpushed to master
Comment #15
sunYotam contacted me privately. As far as I can see, the remaining issues are
i) there's no clear "featured" or "latest official" release, so end-users _will_ download a wrong alpha/dev version (took even myself a few minutes to figure out what the current stable is), and
ii) the download archive contains an arbitrarily named top-level folder instead of simply "jwysiwyg". This is not really a blocker, makes it only harder for end-users, compared to other editor libraries.
Comment #16
jonhattanIt seems jwysiwyg is consolidating since 0.95.
Current version is 0.97.2. They don't upload version packages but they do tagging: https://github.com/akzhan/jwysiwyg/tags. Also archives now contain a directory as jwysiwyg-0.97.2
I think wysiwyg-7.x-2.x can safely start supporting it at 0.95.
Attached patch changes urls (I've set the tags url) and bump minimum version to 0.95. The version detection code still works.
Comment #17
mgiffordComment #18
rpayanmComment #20
twodDon't want to bother with the older versions so going with 0.97 minimum for the future.
Thanks all!