Page MenuHomePhabricator

Automatic references list after page-terminal ordered or bullet list generates unbalanced HTML
Closed, ResolvedPublic

Event Timeline

Jdforrester-WMF renamed this task from Reference list does not function if document ends with bullet points to Automatic references list after page-terminal ordered or bullet list is a bullet list (not an ordered list) and doesn't work.Nov 22 2016, 8:20 PM
Jdforrester-WMF triaged this task as Low priority.
Jdforrester-WMF edited projects, added Cite; removed VisualEditor.
Jdforrester-WMF updated the task description. (Show Details)

The "fun" is because WMF is using tidy. Otherwise there's no fun, because it completely trashes the skin. See this for example

This is heavily impacting us, because we usually have a trailing section on the page with a bullet list listing the page title in other languages, and having a missing references is almost-always causing this weird code.

Another test on WMF wikis with the generated output: https://www.mediawiki.org/wiki/Special:ApiSandbox#action=parse&format=json&oldid=2720811&disablelimitreport=1&disabletidy=1

This is the HTML (or rather tagsoup) code it generates:

<p>Text with references but no references tag<sup id="cite_ref-1" class="reference"><a href="#cite_note-1">&#91;1&#93;</a></sup>
</p>
<ul>
  <li> This is a bullet list
   <div class="mw-references-wrap">
     <ol class="references">
  </li>
</ul>
<li id="cite_note-1"><span class="mw-cite-backlink"><a href="#cite_ref-1">1</a></span> <span class="reference-text">This is a reference</span></li>
</ol>
</div>
Ciencia_Al_Poder renamed this task from Automatic references list after page-terminal ordered or bullet list is a bullet list (not an ordered list) and doesn't work to Automatic references list after page-terminal ordered or bullet list generates unbalanced HTML.Feb 21 2018, 9:12 PM

I overcame this issue on our Wikis buy adding a line break before the reference is inserted:

diff --git a/extensions/Cite/includes/Cite.php b/extensions/Cite/includes/Cite.php
index e44fd50881..9ae5216f23 100644
--- a/extensions/Cite/includes/Cite.php
+++ b/extensions/Cite/includes/Cite.php
@@ -1200,7 +1200,7 @@ class Cite {
                        }
                        $text .= $s . '</div>';
                } else {
-                       $text .= $s;
+                       $text .= "\n".$s;
                }
                return true;
        }

It may be a hacky fix, but it worked for this specific issue for us.

This comment was removed by Alexia.

It looks like a sane fix for this. The previous part of the "if" clause already adds a \n at the end of $text before adding the cite contents.

Would you like to write a patch for that?. Otherwise I can happily write a patch myself for this

Is this still a problem with RemexHtml ?

Is this still a problem with RemexHtml ?

Yup.

Change 585003 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Cite@master] Add a newline in wikitext before autogenerated reflist

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

Change 585003 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Add a newline in wikitext before autogenerated reflist

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

thiemowmde added a subscriber: thiemowmde.

I don't say the change done in the patch https://gerrit.wikimedia.org/r/585003 is wrong. The test cases do indeed make a lot more sense with the patch applied. But: This will most probably change the output of thousands of pages. Have you been able to estimate how many pages will be affected? Is this communicated? Do you plan to mention this in, for example, TechNews? Do our user bases know how to fix templates and pages that look more broken now than before?

@matmarex Commenting here for visibility: I've reverted your patch per @thiemowmde 's note, just to make more space for discussion. Sorry for any inconvenience!

Yeah, no problem, I'll submit it for review again.

@thiemowmde No, I have no idea how many pages would be affected. I would guess that (relatively) very few, since my change only affects the automatically generated reflist added where there is no <references> tag anywhere on the page. There is an old feature request for a tracking category that would contain such pages: T69700.

I do not think that this change could have a negative effect on any page ever, so I'm not planning to announce it anywhere.

Change 585336 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Cite@master] Add a newline in wikitext before autogenerated reflist

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

I'm sorry for the revert. It turns out I missed the fact this only affects pages that don't have a <references /> tag. These should indeed be rare. Not only that. I can not really think of a situation where a user was intentionally using this as a feature. The auto-generated <references /> was appended to the end of the page with no separation. This either does not have any effect, or causes bug, as explained in this ticket.

I got confused because the patch touches so many test cases. Turns out we are lazy and use the auto-generation a lot in these tests. ;-)

Change 594736 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Cite@master] Refactor newline logic for auto-generated <references> sections

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

Change 585336 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Add a newline in wikitext before autogenerated reflist

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