Strange behavior with new plugin call

[svasti]

Just stumbled upon a strange behavior in 1.6.6.
If you call a plugin twice on the same page:

{{{plugincall}}}
{{{PLUGIN:plugincall}}}

the first will not show. If you remove the "PLUGIN:", both will show.

However this stange behavior does not appear, when you change in functions.php the regex line 232 to

Code: Select all

$re = '/{{{(?:PLUGIN:)?([a-z_0-9]+)\s*\(?(.*?)\)?;?}}}/iu';
 

Edit:
More strange things: I am testing a new version of quoteoftheday_XH. However incertain cases it will not show under the old regex , it simply doesn't evaluate. When changing the regex to the above, it will show.

[cmb]

Just stumbled upon a strange behavior in 1.6.6.
If you call a plugin twice on the same page:

{{{plugincall}}}
{{{PLUGIN:plugincall}}}

the first will not show. If you remove the "PLUGIN:", both will show.

Argh! The regex is too greedy. As it's now, it interprets everything from the first {{{ until the {{{PLUGIN: as the start of a single plugin call. Anything between the plugin calls will be discarded as well.

The following regex should solve this problem:

Code: Select all

'/{{{(?:.+:)?([a-z_0-9]+)\s*\(?(.*?)\)?;?}}}/iu'

That's simply based on the fact, that a dot doesn't match newlines, if the `s`modifier is not given.

More strange things: I am testing a new version of quoteoftheday_XH. However incertain cases it will not show under the old regex , it simply doesn't evaluate.

I guess that's basically caused by the same issue: somewhere in the HTML after the plugin call there's a colon.

You can nicely see what's happening behind the scene by inserting the following near the end of the function:

Code: Select all

var_dump($calls, $results);

[svasti]

I guess that's basically caused by the same issue: somewhere in the HTML after the plugin call there's a colon.

yes, there was the plugincall {{{PLUGIN:dlcounter('quoteoftheday_XH_0-9.zip');}}}, which ate the {{{quoteoftheday}}} {{{quoteoftheday 'demo'}}}.

You new regex however breaks legendbox() worse that the old regex. In the old regex legendbox with css-arguments would work, if the first colon was followed by a space. With your new regex all colons in arguments have to be followed by a space.
As legendbox() may take rather long CSS-lines, this is unpractical. Therefore I rather prefer my above stated regex, which is fool prove.

[cmb]

You new regex however breaks legendbox() worse that the old regex. In the old regex legendbox with css-arguments would work, if the first colon was followed by a space. With your new regex all colons in arguments have to be followed by a space.
As legendbox() may take rather long CSS-lines, this is unpractical. Therefore I rather prefer my above stated regex, which is fool prove.

Okay, and this is already being discussed and proposed in viewtopic.php?f=10&t=8458.

Shall we open the voting 1.6.7 now?

[svasti]

Shall we open the voting 1.6.7 now?

Ok, if you don't think something else will come up.

[cmb]

Shall we open the voting 1.6.7 now?

Ok, if you don't think something else will come up.

Karl Fogel writes:

Any software of sufficient size and complexity has an essentially arbitrary number of bugs waiting to be discovered.

Actually, there have been some bugs found prior to the release of XH 1.6.6, but we couldn't fix them in time to not delay the important security patch. So it seems releasing XH 1.6.7 at the end of May is appropriate. Then again, there's a lot to do to prepare the 4th sprint of XH 1.7, so maybe it's best to wait a few weeks and start the XH 1.6.7 voting shortly after the next XH 1.7 one.

[cmb]

To clarify: we would vote on the following change:

Code: Select all

Index: cmsimple/functions.php
===================================================================
--- cmsimple/functions.php	(revision 1610)
+++ cmsimple/functions.php	(working copy)
@@ -229,7 +229,7 @@
 
     $message = '<span class="xh_fail">' . $tx['error']['plugincall']
         . '</span>';
-    $re = '/{{{(?:[^:]+:)?([a-z_0-9]+)\s*\(?(.*?)\)?;?}}}/iu';
+    $re = '/{{{(?:PLUGIN:)?([a-z_0-9]+)\s*\(?(.*?)\)?;?}}}/iu';
     preg_match_all($re, $text, $calls, PREG_SET_ORDER | PREG_OFFSET_CAPTURE);
     $results = array();
     foreach ($calls as $call) {

This would remove support for arbitrary replacements of "PLUGIN" (what might break some sites).

[svasti]

To clarify: we would vote on the following change:

YES

This would remove support for arbitrary replacements of "PLUGIN" (what might break some sites).

It may be or not, we don't know. Maybe these sites don't exist, as this possibility is relatively recent.

The present implementation did brake some sites and is definitely a bug, and it forces me to explain complicated work-arounds on my site. Whatever we do, there may be sites which need small adjustments. IMHO the proposed change gives the least problems for the minimum number of sites.

[cmb]

To clarify: we would vote on the following change:

YES

Implemented (r1652, r1653).