Page MenuHomePhabricator

Fix bug in reparseTableAttributes which is broken in some scenarios
Closed, ResolvedPublic

Description

From T115072#1717106:

The following seems broken in that same function.

// Try to re-parse the attributish text content
var attributishPrefix = attributishContent.txt.match(/^[^|]+\|/)[0];

It sees

<ref>Nick Harris: [http://www.dailymail.co.uk/sport/othersports/article-2323161/Paul-Edwards-takes-UK-Athletics-UK-Sport-Kings-College-London-court-drug-tests.html London's Olympic drugbusters 'misled me for years over the test that got me banned for life'], Daily Mail, 11 May 2013</ref><ref>{{Cite news|url=http://query.nytimes.com/gst/fullpage.html?sec=health&res=9804E1DE113FF93AA15753C1A962958260|work=The New York Times|title=SPORTS PEOPLE: TRACK AND FIELD; Shot-Putter Is Banned|date=1994-10-29|accessdate=2010-05-08}}</ref><ref>UK Anti-doping: [http://www.ukad.org.uk/anti-doping-rule-violations/current-violations/search/P40 Current sanctions]</ref>

and blindly looks for a pipe char. That is a broken test as in this case where the pipe char is found within extension content. So, that needs a smarter test than a blind regexp.

Event Timeline

ssastry raised the priority of this task from to Medium.
ssastry updated the task description. (Show Details)
ssastry added a project: Parsoid.
ssastry subscribed.
ssastry claimed this task.

A lot has happened with the code since 2015. Today, https://en.wikipedia.org/api/rest_v1/page/html/List_of_doping_cases_in_athletics/684239207 seems to render fine and the cell in question shows 3 references [307][308][309] properly. In fact, the code doesn't seem to go down that path right now for non-templated content (as in the case of that page) anyway.

So, I'm going to close this. In case there are more specific real bugs encountered, we can open tickets for them and fix those instances. No use keeping this generic ticket around.