Page MenuHomePhabricator

wrong part->commentEnd set to atributting comments
Closed, ResolvedPublic

Description

Author: a.d.bergi

Description:
To be able to search for headline-closing equal sign even when there are comments between the last = and the linebreak, there are some position values appended to the recent domParts. This works well with the first comment, but as soon as a second one comes, the commentEnd value is set to $wsEnd instead of $endPos.
This breaks the intended behavior when the second comment is followed by some blanks.
See the difference between http://en.wikipedia.org/wiki/Special:ExpandTemplates?generate_xml=1&input=%3D%3D%20X%20%3D%3D%20%3C!--%20--%3E%20%3C!--%20--%3E%20 and http://en.wikipedia.org/wiki/Special:ExpandTemplates?generate_xml=1&input=%3D%3D%20X%20%3D%3D%20%3C!--%20--%3E%20%3C!--%20--%3E (mind the trailing whitespace).
I guess it also should work at
http://en.wikipedia.org/wiki/Special:ExpandTemplates?generate_xml=1&input=%3D%3D%20X%20%3D%3D%20%3C!--%20--%3E%20%3C!--%20--%3E%20%3C!--%20--%3E%20%3C!--%20--%3E%20 , but it doesn't.


Version: 1.17.x
Severity: normal

Details

Reference
bz28642

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:35 PM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz28642.

a.d.bergi wrote:

proposed patch, I guess the $wsEnd is the problem

Attached:

a.d.bergi wrote:

correction

Thanks for the fast handling, but as also Nikerabbit complained in the revision comments, it was untested.
And not only that, it even was wrong. Setting a value before testing it makes not much sense :-(

I also included the same patch for the preprocessor-hash.

Attached:

Could you provide a unit test using FauxRequest and an API call to ExpandTemplates? Even some sample code that I could mangle into a unit test would work.

a.d.bergi wrote:

The regexp for a heading would be

+.+?=+[ \t]*(<!--[\s\S]*?--> *)*[ \t]*

with (fake)linebreaks on both sides. So I can give only some code examples.
I'm going to write a preprocessor test (/phase3/tests/parser/preprocess/), you may use its code for a unit test (or try /phase3/tests/phpunit/includes/parser/PreprocessorTest.php). Unfortunately I have no PHP running anywhere.

I've applied your updated patch in r86709. If you provide an attempt at test, I'll make sure it works.

a.d.bergi wrote:

preprocessor test file

attachment bug28642.txt ignored as obsolete

a.d.bergi wrote:

preprocessor test expection

attachment bug28642.expected ignored as obsolete

a.d.bergi wrote:

Test works at translatewiki.

Can you provide the test as a patch to tests/phpunit/includes/parser/PreprocessorTest.php ?
(and fixing anything it may have broke)

a.d.bergi wrote:

applying the test files

Theres nothing to change in provideCases(), and nothing is broken. The former broken things, like "=== Foo ===<!-- --> <!-- --> \n", were not included there, and "=== Foo ===<!-- --> <!-- -->\n" did already work.
You may extend the test files with these cases like "==<!-- -->= Foo === ", and you may extend the provideCases array with some tabulator cases.

Attached: