Page MenuHomePhabricator

Problem with template transclusion and switching editors
Closed, ResolvedPublic

Description

When testing Discussion Tools at https://en.wikipedia.beta.wmflabs.org/wiki/User_talk:Kaldari?dtvisual=1, I entered {{test}} to test template transclusion (initially in source editing mode). I then switched to visual mode. The transclusion seemed to work OK, but it added an extra line break at the top. When I then switched back to source editing mode, I got the following:

<span></span>
[[File:Placeholder.png|248x248px]]

... rather than {{test}} (what I expected).

Here are screenshots of the progression...
Initial source mode:

Screen Shot 2020-05-19 at 3.24.25 PM.png (472×619 px, 79 KB)

Switched to visual mode:
Screen Shot 2020-05-19 at 3.24.42 PM.png (427×624 px, 90 KB)

Switched back to source mode:
Screen Shot 2020-05-19 at 3.30.20 PM.png (480×690 px, 103 KB)

The source of the Test template can be seen here: https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Template:Test&action=edit

Event Timeline

kaldari renamed this task from Issue with template transclusion and switching editors to Problem with template transclusion and switching editors.May 19 2020, 7:34 PM
kaldari added a project: Editing-team.

Looks like we are messing up the Parsoid HTML for templates during the de-indentation process of switching from source to visual. Will investigate and see if there's an easy solution.

This particular template is multi-line because it has a linebreak after the [[File:]] which will cause it to fail even in the full page wikitext editor:

:{{test}} ~~~~

becomes

image.png (328×317 px, 64 KB)

Notice that my signature (and leading space) has been forced onto a new line, cause it to be <pre> wrapped.

A more well formed template may still cause issues here, as Parsoid puts all the template markers onto the parent definition list item, and we unwrap the list items before putting the HTML into VE.

@cscott @ssastry is it correct that in

:{{test}}

(where test is https://en.wikipedia.beta.wmflabs.org/wiki/Template:Test)

the template attributes get put on the <dl>:

<dl about="#mwt1" typeof="mw:Transclusion" data-mw='{"parts":[":",{"template":{"target":{"wt":"betatest","href":"./Template:Betatest"},"params":{},"i":0}}]}' id="mwAQ">
  <dd>
    <figure-inline ...>...</figure-inline>
  </dd>
</dl><span about="#mwt1">
</span>

?

Change 598847 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/DiscussionTools@master] wt->visual: Don't unwrap template lists

https://gerrit.wikimedia.org/r/598847

The above patch avoids unwrapping templated lists. This means the template isn't destroyed anymore, but it is given an extra ':' prefix, which isn't ideal but is more tolerable. In the long term this would be resolved by our proposed multi-line comment syntax.

This below conflrms the behavior

[subbu@earth:~/work/wmf/parsoid] echo -e ":{{1x|[[File:Placeholder.png|200px]]}}" | php bin/parse.php --body_only
<dl data-parsoid='{"dsr":[0,38,0,0]}'><dd data-parsoid='{"dsr":[0,38,1,0]}'><figure-inline typeof="mw:Transclusion mw:Image" about="#mwt1" data-parsoid='{"optList":[{"ck":"width","ak":"200px"}],"dsr":[1,38,null,null],"pi":[[{"k":"1"}]]}' data-mw='{"parts":[{"template":{"target":{"wt":"1x","href":"./Template:1x"},"params":{"1":{"wt":"[[File:Placeholder.png|200px]]"}},"i":0}}]}'><a href="./File:Placeholder.png"><img resource="./File:Placeholder.png" src="//upload.wikimedia.org/wikipedia/commons/thumb/4/47/Placeholder.png/200px-Placeholder.png" data-file-width="678" data-file-height="841" data-file-type="bitmap" height="248" width="200"/></a></figure-inline></dd></dl>

[subbu@earth:~/work/wmf/parsoid] echo -e ":{{1x|[[File:Placeholder.png|200px]]\n}}" | php bin/parse.php --body_only
<dl about="#mwt1" typeof="mw:Transclusion" data-parsoid='{"dsr":[0,39,0,0],"firstWikitextNode":"DL","pi":[[{"k":"1"}]]}' data-mw='{"parts":[":",{"template":{"target":{"wt":"1x","href":"./Template:1x"},"params":{"1":{"wt":"[[File:Placeholder.png|200px]]\n"}},"i":0}}]}'><dd><figure-inline typeof="mw:Image"><a href="./File:Placeholder.png"><img resource="./File:Placeholder.png" src="//upload.wikimedia.org/wikipedia/commons/thumb/4/47/Placeholder.png/200px-Placeholder.png" data-file-width="678" data-file-height="841" data-file-type="bitmap" height="248" width="200"/></a></figure-inline></dd></dl><span about="#mwt1">
</span>

And yes, it is correct since the newline is now outside the <dl> and so, the template scope expands to include the newline and its sibling, the <dl> tag. . And, looking at the core parser output as well, looks like the newline is outside the <dl> there as well. There is no obvious fix here since if we make the newline part of the <dl> by closing the list after the newline, it will potentially have lots of ramifications since we have fine-tuned the whole DSR computation, tpl-wrapping and other logic with the current structure and not sure what might break.

Ideal solution would be to trim trailing newlines (after expansion and *after* processing include directives which is what is introducing this straggler newline -- the newline is before the <noinclude>) but, not sure what pages will now break where they might be relying on this behavior. But, AFAICT, editors know about this trailing newline preserving behavior since I often see <*include*> tags hugging their surrounding text closely without whitespace and newlines. So, presumably, in this case, we would tell @kaldari to fix his test template but looks like you already added code to handle templated-lists scenario to avoid breakage when such newlines do end up in templates.

Edit: Removed my flippant remark about wikitext & newlines below.

This comment was removed by ssastry.

Thanks @ssastry. Confirmed on beta that this isn't a problem with well-formed templates. {{infobox}} for example works fine.

Change 598847 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] wt->visual: Don't unwrap template lists

https://gerrit.wikimedia.org/r/598847