Last modified: 2013-03-14 17:15:14 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T14906, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 12906 - New parser breaks some template expansion in Semantic MediaWiki
New parser breaks some template expansion in Semantic MediaWiki
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
1.12.x
All All
: Low normal (vote)
: ---
Assigned To: Nobody - You can work on this!
aklapper-moreinfo
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-04 09:03 UTC by Thomas Bleher
Modified: 2013-03-14 17:15 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
A parser function demonstrating the bug (606 bytes, application/x-php)
2008-02-04 09:03 UTC, Thomas Bleher
Details

Description Thomas Bleher 2008-02-04 09:03:36 UTC
Created attachment 4615 [details]
A parser function demonstrating the bug

SVN r30106 breaks a part of the {{#ask}} function of SMW.
I will first describe what this function does and how it breaks, then I will present a simple test-case.

The #ask function has a mode where it takes a template, and a query for certain properties. For each page that matches the given property, it fills out the template with some parameters relating to the page and the query. It then lets the parser preprocess the constructed string and returns it. This worked fine until r30105, under r30106 it breaks in some cases, namely when the template which is to be expanded contains tags like <nowiki>.

The relevant code is in extensions/SemanticMediaWiki/includes/SMW_QP_Template.php::getResultText() and does basically:

        $myparser = clone $wgParser;
        $parser_options = new ParserOptions();
        $result = $myparser->preprocess($text, $wgTitle, $parser_options);

where $text is a string with many templates concatenated (e.g. $text='{{t|1=foo}}{{t|1=bar}}{{t|1=baz}}')

The testcase:
A page Template:Bugtest containing the following line:

<nowiki>Bug</nowiki>

An extension (also attached):
<?php

$wgExtensionFunctions[] = 'efTest_Setup';
$wgHooks['LanguageGetMagic'][] = 'efTest_Magic';

function efTest_Setup() {
        global $wgParser;
        $wgParser->setFunctionHook( 'construct', 'efTest_Construct' );
}

function efTest_Magic( &$magicWords, $langCode ) {
        $magicWords['construct'] = array( 0, 'construct' );
        return true;
}

function efTest_Construct( &$parser, $template = '' ) {
        global $wgTitle, $wgParser;
        $myparser = clone $wgParser;
        $parser_options = new ParserOptions();
        $text = '{{'.$template.'}}';
        $result = $myparser->preprocess($text, $wgTitle, $parser_options);
        return $result;
}

As you can see, {{#construct:Bugtest}} should be the same as just {{Bugtest}}, i.e. it should be a no-op.
Since r30106, however, {{#construct:Bugtest}} displays the literal string '<nowiki>Bug</nowiki>' instead of just 'Bug'.

I found that the function works if the preprocess call is replaced by
        $result = $myparser->preprocessToDom($text, $wgTitle, $parser_options);
        return array( $result, 'isLocalObj' => true );

For SMW, this is not so easy, however, as it currently depends on the preprocessed value being a string (it concatenates it with other strings, for example). Additionally, changing to preprocessToDom would mean that the function would need to check for old vs new parser.

Summary: There should be a way for extensions to properly expand templates, or a flag by which they can tell the parser: "The returned text contains templates, please expand them".
Comment 1 Thomas Bleher 2008-02-08 12:46:57 UTC
Just as an additional small data point: I retested today using the new preprocessor (Preprocessor_Hash) and up-to-date svn (r30692); the result is the same, i.e. it does not depend on the choice of preprocessor (DOM or Hash).
Comment 2 Thomas Bleher 2008-02-09 17:46:56 UTC
OK, changing

  $result = $myparser->preprocess($text, $wgTitle, $parser_options);

to

  $frame = $parser->getPreprocessor()->newFrame();
  $dom = $parser->preprocessToDom( $text );
  $result = $frame->expand( $dom );

fixes my issues. The issue seems to be that Parser::preprocess() calls

  $text = $this->mStripState->unstripBoth( $text );

immediately after replaceVariables() (which essentially does the preprocessToDom() call). By omitting this, the unstrip happens later (I guess somewhere in parse(), but I haven't checked). mStripState is apparently shared between the original and the cloned parser, so this the unstrip works at the later state.

The question now: Should extensions be changed to the code above, or should preprocess() be changed?
Comment 3 Markus Krötzsch 2008-03-11 16:52:27 UTC
I have now implemented the above workaround for Semantic MediaWiki, using PHP's method_exists() to preserve compatibility with MW1.11.
Comment 4 Thomas Bleher 2008-03-14 09:45:03 UTC
JFYI: The #lsth parser function (optionally included in Labeled Section Transclusion) also breaks when including templates. The following patch fixes it for me, though I don't know if this is really correct:

--- a/extensions/LabeledSectionTransclusion/lsth.php
+++ b/extensions/LabeledSectionTransclusion/lsth.php
@@ -92,7 +92,11 @@ function wfLstIncludeHeading($parser, $page='', $sec='', $to='')
     $result = substr($text, $begin_off, $end_off - $begin_off);
   else
     $result = substr($text, $begin_off);
-
+
+  $frame = $parser->getPreprocessor()->newFrame();
+  $dom = $parser->preprocessToDom( $result );
+  $result = $frame->expand( $dom );
+
   return LabeledSectionTransclusion::parse_($parser,$title,$result, "#lsth:${page}|${sec}", $nhead);
 }

A generic fix would be very nice.
Comment 5 Nicolas Brouard 2008-04-08 08:02:17 UTC
Many thanks Thomas! It works for me. I didn't know how to make it work again!
Comment 6 Steve Sanbeg 2008-04-08 19:53:01 UTC
(In reply to comment #4)
> JFYI: The #lsth parser function (optionally included in Labeled Section
> Transclusion) also breaks when including templates. The following patch fixes
> it for me, though I don't know if this is really correct:
> 
> --- a/extensions/LabeledSectionTransclusion/lsth.php
> +++ b/extensions/LabeledSectionTransclusion/lsth.php
> @@ -92,7 +92,11 @@ function wfLstIncludeHeading($parser, $page='', $sec='',
> $to='')
>      $result = substr($text, $begin_off, $end_off - $begin_off);
>    else
>      $result = substr($text, $begin_off);
> -
> +
> +  $frame = $parser->getPreprocessor()->newFrame();
> +  $dom = $parser->preprocessToDom( $result );
> +  $result = $frame->expand( $dom );
> +
>    return LabeledSectionTransclusion::parse_($parser,$title,$result,
> "#lsth:${page}|${sec}", $nhead);
>  }
> 
> A generic fix would be very nice.
> 

OK, I added that patch in r32963, with the method_exists wrapper.
Comment 7 Andre Klapper 2013-03-14 16:55:24 UTC
Thomas Bleher: Is this still something you'd like to see fixed after all those year (and with the workaround from comment 3 in place in SMW), or can this report be closed nowadays?
Comment 8 Thomas Bleher 2013-03-14 17:15:14 UTC
I don't think I care anymore. After all, any code using the parser will have been updated by now :)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links