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)

Comments

twod’s picture

Title: jWYSIWYG 0.92 support » jWysiwyg's official site has moved to a new URL.
Status: Active » Needs work
+++ jwysiwyg.inc	2010-09-26 21:37:18.000000000 +0400
@@ -54,9 +54,9 @@
-  if (preg_match('@([0-9\.]+)$@', $line, $version)) {
+  if (preg_match('@(plugin\s+)?([0-9\.]+)@', $line, $version)) {

The only real change here is that the $ has been removed. We're only interested in the version number, optionally matching plugin\s does 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.

ph0enix’s picture

I have extracted by instruction, so path to editor was sites/all/libraries/jwysiwyg/jquery.jwysiwyg.js

jwysiwyg 0.92 has following version string:

 * WYSIWYG - jQuery plugin 0.92 (arigatou gozaimasu)

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:

* Version string in source file must ended with version number for Drupal integration folks - xeto
twod’s picture

Oh, 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.

ph0enix’s picture

Yes, 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.

twod’s picture

Title: jWysiwyg's official site has moved to a new URL. » jWysiwyg's official site has moved to a new URL + version string change.
Status: Needs work » Needs review
StatusFileSize
new1.09 KB

Ok, 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. =)

twod’s picture

Status: Needs review » Reviewed & tested by the community

Very small change and easy to test, so bumping this to RTBC to give it some more attention.

sun’s picture

Status: Reviewed & tested by the community » Needs review

Wanted 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.

sun’s picture

Priority: Normal » Major
Issue tags: +Release blocker
sun’s picture

sun’s picture

Status: Needs review » Postponed
Issue tags: -Release blocker

No response over there yet, so this will have to wait a bit.

sun’s picture

Title: jWysiwyg's official site has moved to a new URL + version string change. » jwysiwyg: New website and release package structure
Priority: Major » Normal

Reading 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.

frost-nzcr4’s picture

frost-nzcr4’s picture

pushed to master

sun’s picture

Yotam 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.

jonhattan’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Postponed » Needs review
StatusFileSize
new1.11 KB

It 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.

mgifford’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll
Parent issue: » #2343445: Stable release of Wysiwyg module, v7.x-2.4 please
rpayanm’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new1.18 KB

  • TwoD committed d93e895 on 6.x-2.x authored by rpayanm
    - #923460 by TwoD, ph0enix, jonhattan, rpayanm: Fixed jWYSIWYG website...
  • TwoD committed 34287e2 on 7.x-2.x authored by rpayanm
    - #923460 by TwoD, ph0enix, jonhattan, rpayanm: Fixed jWYSIWYG website...
twod’s picture

Status: Needs review » Fixed

Don't want to bother with the older versions so going with 0.97 minimum for the future.
Thanks all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.