Page MenuHomePhabricator

Remove 'missing page parameter' workaround once RESTBase cache & structured discussions content have turned over
Closed, InvalidPublic

Description

When we added the ?page= parameter to paged media, the content in the RESTBase cache was still missing this. We added a hacky workaround to prevent RESTBase from concluding that the page= parameter had been deliberately removed when given an <a href="...."> in a figure which was missing the page parameter.

Once the restbase cache expires, we should remove this workaround, so that an editor can indicate they wish to remove the page parameter from a figure by removing the ?page= portion of the URL. We should also take a look at VE's handling of paged media and ensure it matches the new conventions.

In theory Structured Discussions could also have old page option markup, although the combination of paged media being very rare on talk pages *plus* editing of existing (old) comments being fairly rare, means that the corner case that would cause the page option to be lost is well off in the long tail.

Event Timeline

Do you know when we added the workaround and the HTML version was for it? If it is an older HTML version, it has probably expired in RESTBase. But more than RESTBase, store Flow HTML is the problematic one. That is the source of all b/c hacks in Parsoid that we haven't been able to remove. T209120: Upgrade Parsoid HTML stored in the StructuredDiscussions tables is the associated ticket.

It's in the patch you haven't reviewed yet! That is, the workaround hasn't been deployed yet. This is a reminder task for me for N months from now.

It's in the patch you haven't reviewed yet! That is, the workaround hasn't been deployed yet. This is a reminder task for me for N months from now.

Ok. Maybe edit the description & title to flag the fact that this needs StructuredDiscussions content to be fixed as well (unless we know this wikitext / HTML is not going to be used in structured discussions). And, if applicable, make this a parent task of T209120.

cscott renamed this task from Remove 'missing page parameter' workaround once RESTBase cache has turned over to Remove 'missing page parameter' workaround once RESTBase cache & structured discussions content have turned over.Aug 13 2020, 3:33 AM
cscott updated the task description. (Show Details)
ssastry triaged this task as Medium priority.Aug 13 2020, 10:46 PM
ssastry edited projects, added Technical-Debt; removed VisualEditor.
ssastry moved this task from Needs Triage to Tech Debt / Big changes on the Parsoid board.

Change 776980 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Don't rely on the link element for the page

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

Change 776980 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Don't rely on the link element to roundtrip the |page= option

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

Change 784338 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@master] Bump parsoid to 0.16.0-a6

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

Change 784341 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@wmf/1.39.0-wmf.8] Bump parsoid to 0.16.0-a6

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

Change 784338 merged by jenkins-bot:

[mediawiki/vendor@master] Bump parsoid to 0.16.0-a6

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

Change 784341 abandoned by Arlolra:

[mediawiki/vendor@wmf/1.39.0-wmf.8] Bump parsoid to 0.16.0-a6

Reason:

Will just ride the train next week

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