Page MenuHomePhabricator
Create Task
Maniphest T184466

Live preview doesn’t use new versions of templates embedded in themselves
Closed, ResolvedPublic8 Estimated Story Points
Actions

Description

If I edit a template with live preview turned on, and I preview it after some changes, the inclusions of that very template in the preview show the old version.

Steps to reproduce:

  1. Turn on live preview.
  2. Create and save the page Template:MyTemplate with text:
<includeonly>{{{1|default}}}</includeonly>
<noinclude>{{MyTemplate}}</noinclude>
  1. Open the editing interface for that template again, replace "default" with "new version"
  2. Click on preview.

Expected result:
The preview shows the text "new version".

Actual result:
The preview shows the text "default", i.e. the text in the latest saved version (which may be even later than the current unsaved version, in case of edit conflict).

Details

SubjectRepoBranchLines +/-
Move the setup of fake revision for preview to ContentHandlermediawiki/coremaster+11 -5
Customize query in gerrit

Related Objects
Search...

Event Timeline

Tacsipacsi triaged this task as Medium priority.Jan 8 2018, 6:04 PM
Tacsipacsi created this task.
matmarex added a comment.Jan 8 2018, 6:12 PM

Hmm, does this work for non-live preview?

matmarex removed projects: WikiEditor, Tracking-Neverending.Jan 8 2018, 6:13 PM
Umherirrender added a project: MediaWiki-Action-API.Jan 8 2018, 6:29 PM
Umherirrender subscribed.

Yes, EditPage sets up a fake revision (ParserOptions::setupFakeRevision)

Not sure, if that fits in api's action=parse preview handling and how it affect caching

Anomie subscribed.Jan 8 2018, 6:38 PM

action=parse doesn't really have a "preview" mode, although it does have a flag to set ParserOptions's IsPreview flag true.

It may be better to file a subtask to request that functionality in ApiParse, leaving this task for having live preview actually use it once it's available.

matmarex unsubscribed.Jan 8 2018, 6:43 PM
Anomie moved this task from Unsorted to Needs details or plan on the MediaWiki-Action-API board.Jan 9 2018, 2:52 PM
• 238482n375 set Security to Software security bug.Jun 15 2018, 8:06 AM
• 238482n375 added a project: acl*security.
• 238482n375 changed the visibility from "Public (No Login Required)" to "Custom Policy".
• 238482n375 subscribed.

SG9tZVBoYWJyaWNhdG9yCk5vIG1lc3NhZ2VzLiBObyBub3RpZmljYXRpb25zLgoKICAgIFNlYXJjaAoKQ3JlYXRlIFRhc2sKTWFuaXBoZXN0ClQxOTcyODEKRml4IGZhaWxpbmcgd2VicmVxdWVzdCBob3VycyAodXBsb2FkIGFuZCB0ZXh0IDIwMTgtMDYtMTQtMTEpCk9wZW4sIE5lZWRzIFRyaWFnZVB1YmxpYwoKICAgIEVkaXQgVGFzawogICAgRWRpdCBSZWxhdGVkIFRhc2tzLi4uCiAgICBFZGl0IFJlbGF0ZWQgT2JqZWN0cy4uLgogICAgUHJvdGVjdCBhcyBzZWN1cml0eSBpc3N1ZQoKICAgIE11dGUgTm90aWZpY2F0aW9ucwogICAgQXdhcmQgVG9rZW4KICAgIEZsYWcgRm9yIExhdGVyCgpFVzZSC3IERpc2NsYWltZXIgtyBDQy1CWS1TQSC3IEdQTApZb3VyIGJyb3dzZXIgdGltZXpvbmUgc2V0dGluZyBkaWZmZXJzIGZyb20gdGhlIHRpbWV6b25lIHNldHRpbmcgaW4geW91ciBwcm9maWxlLCBjbGljayB0byByZWNvbmNpbGUu

Dzahn removed a project: acl*security.Jun 15 2018, 10:48 AM
Dzahn changed the visibility from "Custom Policy" to "Public (No Login Required)".
Dzahn removed a subscriber: • 238482n375.
Restricted Application added a project: acl*security. · View Herald TranscriptJun 15 2018, 10:48 AM
Dzahn removed a project: acl*security.Jun 15 2018, 10:49 AM
MGChecker moved this task from Backlog to Live preview on the MediaWiki-Page-editing board.Nov 15 2018, 10:21 PM
Aklapper removed subscribers: Anomie, • TrevorParscal.Oct 16 2020, 5:02 PM
Umherirrender unsubscribed.Feb 27 2021, 9:51 PM
Nardog subscribed.Apr 2 2021, 9:31 AM
stjn subscribed.May 28 2021, 10:27 AM
• NRodriguez added a project: Community-Tech (CommTech-Sprint-10).Sep 23 2021, 3:56 PM
MusikAnimal set the point value for this task to 8.Sep 23 2021, 5:36 PM
Daimona updated the task description. (Show Details)Sep 24 2021, 5:43 PM
ldelench_wmf edited projects, added Community-Tech (CommTech-Sprint-11); removed Community-Tech (CommTech-Sprint-10).Oct 11 2021, 2:00 PM
ldelench_wmf moved this task from CommTech-Sprint-11 to CommTech-Sprint-12 on the Community-Tech board.Oct 25 2021, 8:04 PM
ldelench_wmf edited projects, added Community-Tech (CommTech-Sprint-12); removed Community-Tech (CommTech-Sprint-11).
Samwilson added subscribers: Anomie, Samwilson.Nov 8 2021, 3:38 AM
In T184466#3883829, @Anomie wrote:

action=parse doesn't really have a "preview" mode, although it does have a flag to set ParserOptions's IsPreview flag true.

It may be better to file a subtask to request that functionality in ApiParse, leaving this task for having live preview actually use it once it's available.

Live preview is already using the preview flag for action=parse.

Anomie unsubscribed.Nov 8 2021, 4:42 PM
• Elitre unsubscribed.Nov 8 2021, 5:05 PM
• NRodriguez added a project: Realtime Preview.Nov 8 2021, 7:11 PM
ldelench_wmf moved this task from CommTech-Sprint-12 to CommTech-Sprint-13 on the Community-Tech board.Nov 8 2021, 9:55 PM
ldelench_wmf edited projects, added Community-Tech (CommTech-Sprint-13); removed Community-Tech (CommTech-Sprint-12).
Daimona raised the priority of this task from Medium to Needs Triage.Nov 9 2021, 11:08 AM
Daimona added a project: Platform Engineering.
Daimona subscribed.

Sam and I discussed this today, here is a summary.

First, it seems like core doesn't provide a dedicated component for page previews. If you want to get the HTML of a preview, you have to use APIParse (client) or directly Parser (from within MW code). In doing so, you should set the IsPreview ParserOptions flag, but that is not enough, as this bug report shows. Instead, what EditPage.php and TemplateSandbox currently do is set a fake revision with the new content of the page that will be used by the parser. Even if we don't want to change that, this logic could be at least moved to a new service responsible for building previews, so that we don't need to propagate the fake revision hack to every place where we want an actual preview (which includes APIParse when the "preview" option is set).

Additionally, the clients may get access to the new service via a separate module (e.g. APIPreview, instead of APIParse), which might allow easier addition of new features. However, we didn't look closely at the APIParse to determine whether this would be easy to do and how much duplication we might end up introducing.

All in all, this seems more of a core bug, which might be stated as "The 'preview' option to APIParse is not equivalent to normal page preview", and if we solve that, there's nothing in LivePreview that we need to change.

I'm tagging CPT for input on the proposed service, as I'm not entirely sure about where it should live, what entry points it should have, what it should return (ParserOutput vs HTML -- the latter would ensure consistency across all consumers but we may not need that) and what it should take (in particular, whether callers may pass additional parser options). In my mind, it would be something like this:

class PreviewRenderer { // Naming is hard.
  __construct( /* ContentTransformer, ContentRenderer, ... */ )
  getPreviewOutput( ProperPageIdentity $page, UserIdentity $user, Content $newContent, ParserOptions $extraOptions = null ): ParserOutput
}
• NRodriguez moved this task from CommTech-Sprint-13 to Up Next on the Community-Tech board.Nov 18 2021, 4:22 PM
• NRodriguez edited projects, added Community-Tech; removed Community-Tech (CommTech-Sprint-13).
• NRodriguez removed a project: Realtime Preview.Mar 28 2022, 11:25 PM
gerritbot added a comment.Apr 5 2022, 5:53 AM

Change 777193 had a related patch set uploaded (by Func; author: Func):

[mediawiki/core@master] Move the setup of fake revision for preview to ContentHandler

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

gerritbot added a project: Patch-For-Review.Apr 5 2022, 5:53 AM
Func subscribed.Apr 5 2022, 6:16 AM
gerritbot added a comment.Aug 8 2022, 2:51 PM

Change 777193 merged by jenkins-bot:

[mediawiki/core@master] Move the setup of fake revision for preview to ContentHandler

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.25; 2022-08-15).Aug 8 2022, 3:00 PM
Maintenance_bot removed a project: Patch-For-Review.Aug 8 2022, 3:30 PM
TheresNoTime claimed this task.Aug 17 2022, 7:35 PM
TheresNoTime subscribed.

Pending figuring out if there's still work remaining here (any suggestions would be appreciated @Func!)

Func added a comment.Aug 17 2022, 11:41 PM
In T184466#8163113, @TheresNoTime wrote:

Pending figuring out if there's still work remaining here (any suggestions would be appreciated @Func!)

Where are you testing on? For example, enwiki is on 1.39.0-wmf.23 (08:09, 2 August 2022), which didn't get the patch. And beta clusters work fine for me.

TheresNoTime removed TheresNoTime as the assignee of this task.Aug 29 2022, 4:42 AM
In T184466#8163741, @Func wrote:
In T184466#8163113, @TheresNoTime wrote:

Pending figuring out if there's still work remaining here (any suggestions would be appreciated @Func!)

[...] beta clusters work fine for me.

Which suggests that the patch works as expected, I believe? 🙂

Func closed this task as Resolved.Aug 29 2022, 4:47 AM
Func claimed this task.

Tested on prod, resolved.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL