Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks’s picture

Status: Active » Needs work
kaidawai’s picture

which browser(chrome?), which other modules?

attiks’s picture

Firefox12, Chrome 18

enabled modules:


coffee
ctools
flot
flot_example
clientside_validation
clientside_validation_example
clientside_validation_fapi
clientside_validation_form
clientside_validation_html5
clientside_validation_webform
context
decorator
views_decorator
coder
coder_review
devel
fapi_validation
testswarm
testswarm_forms
testswarm_tests
defaultcontent
features
testswarm_forms_radios
elements
syntaxhighlighter
socialmedia
googleanalytics
views
views_ui
webform
widgets
attiks’s picture

BTW: file is included only once

kaidawai’s picture

module "syntaxhighlighter" brings xregexp with it so it is actually loaded twice. t
his is why adding the library is optional. just not adding it should imho resolve it.
but i check again.

kaidawai’s picture

it is not included once. syntacshighliter loads it within its own syntaxhighliter javascript (they bundled it in one js file).

kaidawai’s picture

Added some settings and verbosity to the description.
settings:
a) skip loading: it won't try load the lib even if user puts it into libraries.
b) force plugin loading: handles the case other module loads the lib without plugins, but user needs extemded unicode support like \p{L}.

Still don't understand where new commits end up in the git. I see them delayed or look at the wrong place. I get that sorted out one day. Meanwhile i did a diff against my lokal files and made it git style -p1. Hope it applies.

attiks’s picture

Status: Needs work » Closed (fixed)

@kaidawai the problem is fixed in version 2, see https://github.com/slevithan/XRegExp. So I changed the link on the settings page.

kaidawai’s picture

did you test it? because changing the version thi module loads doesn't change the version syntxhighlighter has included(or any other module for that matter).
i suggest: do both keep the additional settings just in case /and/ use the link to version 2?

kaidawai’s picture

Status: Closed (fixed) » Needs review
FileSize
3.17 KB

a git pull now gave me the new versions so it seems git is sometimes delayed...
atached is a patch against git - just in case.

kaidawai’s picture

Status: Needs review » Needs work
FileSize
6.6 KB

well, now i did some testing with it.
- first off, sorry, in my patch i got the naming of fields wrong.
- but 2.0 doesn't work with syntaxhighliter and other issues as well

syntaxhighlight has 1.x included.
so only if you are lucky it is loaded first and 2.0 second. in this case 2.0 will recognice that it is loaded and gracfully stop installing itself. if it is loaded the other way round 2.0 first and the 1.x in syntaxhighlighter second, you are out of luck and you still get the error.
also if somone wants unicode the 2.0 libs won't work with the 1.x.

besides that there are severe differences between 1.x and 2.0:

a) naming is different with 2.0(it won't find the lib)
b) directory structure is different
c) it does some checks prior versions didn't.

so... i sat down and changed the lookup for the files.
also to accept the "....all.js", so a user can just unpack the archive without guessing where the files go.

2.0 throws an exception if it finds an unknown modifier, prior version just ignored them. -> i filter them.

doing this i saw the 'g' modifier you(?) added, which is imho wrong. php has two functions 'preg_match' and 'preg_match_all'. with 'g' it becomes equivalent to 'preg_match_all' but the ajax callback uses 'preg_match' (that is no 'g' modifier). so i removed it again. please correct me if i am wrong...

i put a try catch arround it to handle exception gracefully with fallback.

i removed the else clause at the end of the function that contained the last return to let the exption catch through to the delegation. (such unnecessary else branches will anyway result in a warning on some browsers for it disturbs the flow analysis..)

still not tested enough but now it works with syntaxhighliter but the oposite round: only with 1.x libs otherwise you may see conflicts. just straight forward. if you have syntaxhighlighter(or a another module), don't add the library a second time. if you need the unicode plugins check "force plugins" to add them to the lib syntaxhighlither has included. but they need to be compatible with the version syntaxhighliter uses.
if you see the confict check "don't load library."

this way round it works. but 2.0 and 1.x together ends up in a conflict.

see patch.

kaidawai’s picture

ok today i looked at the "new" version 3.0.83 of the syntaxhighlighter script(the js lib syntaxhighlighter module uses) and it is worse than i thought. in shCore you'll find:

-----
* XRegExp 0.6.1
 * (c) 2007-2008 Steven Levithan
 * <http://stevenlevithan.com/regex/xregexp/>
 * MIT License
....
// prevent running twice, which would break references to native globals
if (!window.XRegExp) {
-----

so this is not even 1.x let alone 2.0., but good news is, that it is only guarded but doesn't throw an error if it is already there(no matter which version).
bad news is, that if syntaxhighliter lib is installed first, possible plugins of 2.0 will fire an error saying that they don't like the version.

looking at it, i think, this all is actually rather something the syntaxhighlighter module concerns and less this project. and not even the drupal syntaxhighliter project but genericaly the script from Alex Gorbatchev upstream.

it was a good idea to switch to 2.0 as it will anyways be stable soon. but switching to 2.0 just doesn't solve the syntaxhighlighter issues(this particular issue).

this said, i think, it should be enough to give the users who want and need syntaxhighliter a hint, that they have to use the old libs that come with syntaxhiglither. i can make it work with that script but resolving the issues of another (outdated) script in another module is a little out of scope, isn't it?

and that it does: clientside_validation can use the other script instead of the (better) new one and works with it(only of course with the given limitations of the other script).

[independend of this particular issues the switch to 2.0 requires some of the changes i made in last patch, other wise 2.0 will emit exeptions and the like ... and 2.0 needs more testing. but that all is independent of this issue. ]

attiks’s picture

FileSize
50.6 KB

@kaidawai i did some more thinking, and it's not really up to this module to solve this, so I propose we don't do any fancy tricks.

I think it's easier to build a new drupal module to handle this, so users can easily select which addons they want to use, and clientside_validation doesn't have to handle this. The settings for the new module will look like:

  1. None, meaning it's loaded in an other way (ex syntaxHighlighter)
  2. Version 1.5 + addons
  3. Version 2 + addons

What do you think?

i1584902-13.png

BTW: Syntax highlighter use 1.5.1, see https://github.com/alexgorbatchev/SyntaxHighlighter/blob/master/scripts/...

kaidawai’s picture

i see, the one on git uses 1.5. nice! the above codesnippet was from the newest release(version see above).
still doesn't make things easier for us though as that isn't 2.0 either.
i like your idea. it is comfort deluxe. looks nice!

regarding this issue:
a) give the user the option to choose none(that is use a otherwise supplied library) - guess you do this anyways.
b) if you want to be realy, realy nice, give the user some option "plugins only" or something. (like you do)

but great!

[this post was edited only edited to avoid misunderstanding it was and is the "comfort deluxe" draft ]

attiks’s picture

I created http://drupal.org/project/xregexp_api, further discussion about the UI in #1589562: UI.

Jelle_S’s picture

Clientside validation now supports the XRegExp API Module when it's installed.

Jelle_S’s picture

Status: Needs work » Fixed
attiks’s picture

try .. catch added
removal of unknown modifiers added
fapi_validatin fixed

slevithan’s picture

I'm the author of XRegExp. A few notes:

XRegExp v2.0.0 is now final. I've also made minified versions of xregexp.js and xregexp-all.js available at http://xregexp.com/xregexp-min.js and http://xregexp.com/xregexp-all-min.js . Just like xregexp-all.js, xregexp-all-min.js includes all the latest addons:

XRegExp Unicode Base v1.0.0
XRegExp Unicode Categories v1.2.0
XRegExp Unicode Scripts v1.2.0
XRegExp Unicode Blocks v1.2.0
XRegExp Unicode Properties v1.0.0
XRegExp.matchRecursive v0.2.0
XRegExp.build v0.1.0
XRegExp Prototype Methods v1.0.0

I recommend that you keep the list of available XRegExp versions simple. Something like:

- XRegExp v1.5.1
- XRegExp v2.0.0
- XRegExp v2.0.0 plus addons

Also, I recommend always providing minified versions. When gzipped, the v2.0.0 xregexp-min.js is just under 3.5 KB, and xregexp-all-min.js is just under 23 KB. (Most of the additional weight comes from the Unicode addons. The other addons are small.)

As for SyntaxHighlighter, I've submitted a pull request at https://github.com/alexgorbatchev/SyntaxHighlighter/pull/121 to upgrade it from XRegExp v1.5.1 to v2.0.0. Perhaps you should add a comment there encouraging Alex to accept the commits.

As for all the issues revolving around loading XRegExp twice in the same frame, based on the above discussion I've added an item to the XRegExp roadmap noting that this should be simplified in XRegExp v2.1.0 (see https://github.com/slevithan/XRegExp/wiki/Roadmap ). Essentially, I plan to let XRegExp override previously loaded instances, so that going forward, whatever is the last version loaded will win.

Feedback is welcome.

attiks’s picture

@slevithan thanks for stopping by, we resolved most issues regarding the different versions by creating http://drupal.org/project/xregexp_api, we will add support for the minified versions #1595142: Add minified versions as well

attiks’s picture

FileSize
17.15 KB

Supported downloads

i1584902-21.png

Status: Fixed » Closed (fixed)

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