Page MenuHomePhabricator

{{REVISIONID}} returns null, causing templates to be rendered in Preview mode (when using REST API)
Closed, ResolvedPublic

Description

Here are two screenshots of my sandbox, one from my laptop and one from my phone. The page's content, at the time of the screenshots, was

{{#if:{{REVISIONID}}|If you are seeing this message, then <code><nowiki>{{#if:{{REVISIONID}}}}</nowiki></code> returns positive (and this revision's ID is {{REVISIONID}}).|If you are seeing this message, then <code><nowiki>{{#if:{{REVISIONID}}}}</nowiki></code> returns negative. Here's what happens if you type in <code><nowiki>{{REVISIONID}}</nowiki></code>: {{REVISIONID}}.}}
Laptop 'shot:Mobile 'shot:

This is somewhat important, as it b0rks any modules that assume that no REVISIONID means the page is being previewed, most notably Module:Check for unknown parameters on enwiki. Here is a screenshot of the page that caused me to notice this bug:

@Reedy, adding you since we chatted briefly about this on IRC.

Event Timeline

Restricted Application added subscribers: jeblad, Aklapper. · View Herald Transcript
PinkAmpersand triaged this task as High priority.Sep 5 2017, 12:08 AM

Since this is affecting the display of a large number of en.wp pages, I've boldly set it as high priority. My apologies if I shouldn't have.

Jdlrobson moved this task from Needs triage to Triaged on the Mobile board.Sep 5 2017, 7:46 PM
Dbrant added a subscriber: Dbrant.

Tagging Parsoid, since that's where the issue seems to originate: https://en.wikipedia.org/api/rest_v1/page/html/Iain_Banks

Dbrant renamed this task from {{REVISIONID}} returns null on Android app to {{REVISIONID}} returns null, causing templates to be rendered in Preview mode (when using REST API).Sep 6 2017, 1:56 PM
bearND added a subscriber: bearND.Sep 13 2017, 10:46 AM

While I can still repro the behavior on the sandbox page in the Android app when RESTBase/MCS is enabled, I don't see the error output on the [[Iain Banks]] page anymore. Not sure what's changed.

This is only an problem when useBatchAPI is enabled (as in production), since expandtemplates is otherwise passed the revid (T73306)

https://github.com/wikimedia/parsoid/commit/038195c9406bda4a3e57d29887253a42631f80b4

vs

https://github.com/wikimedia/parsoid/blob/master/lib/mw/Batcher.js#L217-L237

Arlolra claimed this task.Sep 13 2017, 1:38 PM

Change 377782 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] T174977: Pass revid to batcher for preprocessing

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

Izno added a comment.Sep 13 2017, 3:12 PM

Wouldn't this be fixed by applying on-wiki the magic word/Lua (was Lua added?) from T137900: Deal with poor edit stash hit rate due to Lua modules using {{REVISIONID}}?

Change 377782 merged by jenkins-bot:
[mediawiki/services/parsoid@master] T174977: Pass revid to batcher for preprocessing

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

Wouldn't this be fixed by applying on-wiki the magic word/Lua (was Lua added?) from T137900: Deal with poor edit stash hit rate due to Lua modules using {{REVISIONID}}?

That was abandoned, no? The preferred solution was for the module to switch to using the existing warning system, as in T137900#2384230 (but note there's an objection about locational context missing from that api).

Regardless, the patch provided here will fix rendering for all uses of that hack.

Arlolra closed this task as Resolved.Sep 13 2017, 4:00 PM

Change 377942 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/core@master] API: Pass revid when parsing text

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