User:Daniel Kinzler (WMDE)/MCR-PO

While refactoring the MediaWiki storage layer for MCR, questions have arisen regarding the handling of multiple slots in ParserOutput.

This document is intended for discussing the requirements and constraints, and the possible solutions.

Please comment and discuss inline.

Baseline
Goal: provide an MCR-capable interface for updating pages, with a minimum of refactoring of adjecant code. The implementation doesn't need to fully support MCR yet.


 * Use ContentHandler::getParserOutput to generate output for each slot
 * A true "baseline" doesn't require this. You only list it here because of the extra requirements you include below. Anomie (talk) 20:32, 26 February 2018 (UTC)
 * Use Content::getSecondaryDataUpdates to get data updates for each slot
 * A true "baseline" doesn't require this. This seems aimed at "SDC wants two slots blindly concatenated". Anomie (talk) 20:32, 26 February 2018 (UTC)
 * Update links tables with information from all slots. In the future, links tables may also record which slot references what resource.
 * A true "baseline" doesn't require this. This seems aimed at "SDC wants two slots blindly concatenated". Anomie (talk) 20:32, 26 February 2018 (UTC)
 * It's aimed at "SDC wants to be able to place the output of two slots freely on a page", along with "SDC wants to re-use the Wikidata representation and editing interface for the structured output", which I just confirmed as requirements with Mark and Ramsey. Plus "it seems really useful to be able to use output for different slots independently of each other for various use cases", which I confirmed with James. Do you disagree with these things even being requirements? -- Daniel Kinzler (WMDE) (talk) 18:32, 28 February 2018 (UTC)
 * Which doesn't change the fact that a true baseline doesn't require it. Just that SDC's requirements can't be satisfied by a true baseline that doesn't blindly concatenate slots. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * There is no intention for the baseline patch to concatenate HTML. The patch generates POs for all slots, uses them to register secondary data, and then throws the POs away: the code for generating the "combined" PO currently just uses the main slot's PO.
 * One could argue that secondary data (e.g. pagelink rows) should only be added for content in the ParserCache, since we use pagelinks to purge the parser cache. But we also use it for whatlinkshere, to find (and change) references to pages. You'd want to find e.g. template documentation linking to a page in whatlinkshere, even if we end up not showing the documentation in the default view, right?
 * But we can push back the decision on what secondary data gets tracked when, and only do it for the main slot in the initial patch. I'd still want the POs for all slots to be in the PreparedEdit - we will need them in some shape or form anyway. -- Daniel Kinzler (WMDE) (talk) 11:12, 2 March 2018 (UTC)
 * Why wouldn't we show the template documentation in the default view? We do currently, at least on enwiki. I also note that we already get complaints when parser functions cause unexpected links on WhatLinksHere, we'd want to consider that before potentially making it worse by blindly adding links from undisplayed slots. "we will need them in some shape or form anyway" Anomie (talk) 14:34, 2 March 2018 (UTC)
 * Store a single ParserOutput for the revision in the ParserCache. Can be just the ParserOutput for the main slot for now, but will have to somehow contain all information from all ParserOutputs of all slots eventually. The code composing that combined output may be different for different "kinds" of pages, and may be defined by an extension. The mechanism for that doesn't need to be decided at this stage, though.
 * It won't necessarily have to contain all information from all slots. Even if it does contain some information from a slot, that need not be all the information. Trivial example: if we add a "metadata" slot to hold categories and such, there's no need for the main ParserOutput to contain whatever HTML might be generated to view that metadata. Anomie (talk) 20:32, 26 February 2018 (UTC)
 * If we want to have it in the ParserCache, it has to be in the PO. Or we re-design the ParserCache. -- Daniel Kinzler (WMDE) (talk) 18:32, 28 February 2018 (UTC)
 * The key word there is "if". Do we actually want random unused HTML and other content in the main PO, though? Anomie (talk) 17:25, 1 March 2018 (UTC)
 * It's not random, nor unused, not blindly concatenated. The ParserCache would have the HTML as shown in the standard view. And the HTML will in the future be explicitly composed from the POs of the individual slots, not blindly concatenated.
 * We may want to have the per-slot HTML in the ParserCache, using separate keys or in a nested object, for optimized re-rendering. But that's not part of the initial patch. -- Daniel Kinzler (WMDE) (talk) 11:12, 2 March 2018 (UTC)
 * The use of ApiStashEdit can be restricted to the main-slot-only case for now.
 * Only if EditPage is restricted to the main slot only. Anomie (talk) 20:32, 26 February 2018 (UTC)
 * No, it could just not use stashing if it's being used for multiple slots. This will be rare anyway, at least in the beginning. -- Daniel Kinzler (WMDE) (talk) 18:32, 28 February 2018 (UTC)
 * I suppose, although that would be a performance regression for the cases ApiStashEdit is trying to speed up. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * Sure, but it would still work for 99.9% of the edits. Anyway, nothing keeps us from changing ApiStashEdit when refactoring EditPage to allow multiple slots to be edited at once. I'm just saying that it's not a hard requirement. -- Daniel Kinzler (WMDE) (talk) 11:12, 2 March 2018 (UTC)

This implies that we need access to POs for individual slots and a combined PO during saving. This does not imply anything about the structure of the PO that gets written to the ParserCache or shown in a preview.


 * Anomie, Daniel Kinzler: just making sure that I understand what is the disagreement here: Brad would prefer an initial implementation in the spirit of T174039 (use the MCR interface, but completely ignore and/or error out on non-main slots), while Daniel would like to pull in enough functionality to make the SDC use-case work, right? The former seems like a more obvious initial goal, so is some time constraint the motivation to do more at once? Or are you worried that doing the change in small steps might lead into some architectural cul-de-sac? --Tgr (WMF) (talk) 10:20, 1 March 2018 (UTC)
 * No, that's not the issue. The first patch will fail on non-main slots, but we'll have to support them for SDC, so support will be added over the next few weeks. The disagreement is instead:
 * Anomie would like to keep the main slot "the page content", and have other slot content as ancillary that can be used by the main slot. If need be, hooks could be used to provide some kind of "implicit transclusion" that attaches information from the other slots to the main slot's PO. That implicit transclusion would be controlled by hooks. It's unclear to me how multiple hook handlers would interact to compose the final HTML for the main slot's PO.
 * My idea is to have a dedicated facility for composing a "combined" ParserOutput for all slots. That "PageOutputComposer" would be bound to page type (e.g. File pages) and could be overwritten by extensions. But the only thing that is blocking the initial patch is the question whether WikiPage (or rather, the new PageUpdater) should know about a "combined" ParserOutput that is separate from the stand-alone output of the main slot.
 * Below, Anomie talks about "pre-implicit-transclusion output" vs the "post-implicit-transclusion output". This sounds very similar to that I have in mind. The discussion how output is composed, and what component owns the composition, still needs to be had, but it should not block the initial patch, I think. As far as I understand the issue, blocker for this patch is really that I found it useful, for practical reasons, to have the distinction between "main only" and "combined" PO explicit the code. -- Daniel Kinzler (WMDE) (talk) 14:00, 1 March 2018 (UTC)
 * My problem with this section is that it's neither a baseline that ignores or throws errors for non-main slots nor a step towards a functional implementation. Instead it's a hacky implementation that would minimally work for SDC but makes questionable choices that may leave us with tech debt to clean up when the actual implementation is done, depending on what that actual implementation looks like. My problem with Daniel's desire to have a merged PO that contains every slot's individual PO is that it wastes work generating (and space saving) the PO for every non-main slot even if it's not actually required. And, as I said elsewhere, my problem with Daniel's idea of having a combining class per page is that you can't have more than one of them, meaning only one extension can reasonably define extra slots for any particular page. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * Let's differentiate between two things: the current baseline patch, and the minimum requirements for SDC.
 * The current patch doesn't do any combining of POs. It does generate all POs, to register secondary data. That could be considered wasteful if the only need for registering things in the links tables was purging the parser cache. But that is not the only use of that data. But even if it was - we can assume that typically, information from all slots makes it into the default view somehow - that means we need to purge & re-render whenever anything that any slot depends on changes. So we need to run the DataUpdates for all slots.
 * For SDC, we need some mechanism for composing the HTML, and putting the composed HTML into the ParserCache. That does not require nested POs, though we may want them for optimized re-rendering. We don't have to decide that now, and we don't have to decide on the composition mechanism now. We don't even *have* to have the secondary data tracking (links table updates) for all slots in the baseline patch, but we'll need it for SDC.
 * I don't see what is hacky, and what needs to be cleaned up. I find a clear separation of composed output and main slot output helpful, both conceptually and in the code. And I see no way around generating POs for all slots, at least whenever the respective slot changes. We could optimize by generating POs with no HTML, getParserOutput supports this in theory, though I don't many implementations honor that flag. -- Daniel Kinzler (WMDE) (talk) 11:12, 2 March 2018 (UTC)
 * If you still don't see any way around generating POs for all slots, then I may as well give up. I've already said how to do it multiple times on this page. Anomie (talk) 14:34, 2 March 2018 (UTC)

Multi-Slot View
Model use case: Structured Data on Commons (SDoC). Single-slot viewing should also be possible.


 * Allow the rendering of one slots to depend on content of all slots. (Possible restriction: only allow the main slot to depend on other slots. This would allow the main slot to depend on the rendering of other slots, which could otherwise lead to circular dependencies). This needs ContentHandler::getParserOutput to have access to all slots.
 * Allow Article::view (or a replacement of that method) to show the content of each slot, rendered individually, in separate sections of the page. In particular, allow a rendering of the structured data to be shown that is independent of the wikitext, rendered in the user's interface language. That rendering also serves as the editing interface for structured data, just like on a Wikidata page.
 * This, IMO, should be a non-standard view. And it may not need to display all slots at once, versus showing individual slots. Anomie (talk) 20:32, 26 February 2018 (UTC)
 * I asked that question to the MultiMedia team. The result was that they want to at least have the option to have the standard view serve the full HTML of both slots. And it seems likely that this will be the case at least for the initial roll-out. -- Daniel Kinzler (WMDE) (talk) 18:39, 28 February 2018 (UTC)
 * What is meant by "standard" here? That no extension is customizing the rendering, or that the user is not customizing the request (so e.g. the request is for  and not  )? If it's used in the first sense, why is it important for the MM team what the standard behavior is? They are, after all, working on an extension to customize the rendering of file pages. --Tgr (WMF) (talk) 10:19, 1 March 2018 (UTC)
 * The "standard" view is, as opposed to   or  .   would be the same as the default view with Brad's approach, but could provide a different view with my approach.   isn't really a thing in either model: with my approach, all slots would typically (but not necessarily) be visible in the standard view, while in Brad's approach, the standard view is the same as the main slot view, which may have content from other slots transcluded, implicitly or explicitly. -- Daniel Kinzler (WMDE) (talk) 14:14, 1 March 2018 (UTC)
 * Let's not play Telephone. Why can't you ask them these questions here and have them respond here? I have a suspicion that your question and their answer may have confused "the standard view for SDC should include both the wikitext and wikibase output" with the more general question of whether we actually need the ability to have a view that directly shows the HTML of all slots, including slots with HTML that isn't included in the standard view. OTOH, even if we don't need it, it may be simple enough to include it anyway (with, or perhaps only via enumerating all slots like  ). Anomie (talk) 17:25, 1 March 2018 (UTC)
 * Indeed, I did send them an email asking them to comment here.
 * The question of what goes into the standard view is mainly what goes into the ParserCache, what goes into the web cache, and how much coding is needed to make that happen.
 * You are asking the more general question of whether we actually need the ability to have a view that directly shows the HTML of all slots, including slots with HTML that isn't included in the standard view. I don't think we need such a view. I think the standard view should be composed of whatever bits of the content of all slots "the composer" (whatever that may be) sees fit. And I believe we need to track secondary data for all slots, even if not shown in the standard view, for stuff like whatlinkshere (ideally, we could differentiate in the links tables - this is where we get into the discussion of having a separate mechanism for dependency tracking for purging; I'm all for it, and I'm working on a plan with Marko). I do not think we need a view that always shows all slots. I think that would be sensible to have that as the default behavior of the standard view, but I don't feel strongly about that bit.
 * So, again: we need all separate PO for tracking references while processing an edit. We want HTML for some slots combined somehow in the ParserCache and CDN for the standard view. We don't necessarily need a view that gives HTML of all slots. We do need separate views showing the HTML for each slot. -- Daniel Kinzler (WMDE) (talk) 11:23, 2 March 2018 (UTC)
 * The code composing that combined output, when constructing and/or when using the PO, will be different for different "kinds" of pages, and may be defined by an extension. In the SDoC case, it will be specific to the File namespace, and will be supplied by the WikibaseMediaInfo extension.
 * This is an assumption that need not hold, and either limits the combining to a single extension per page or requires different extensions to hook into or extend each other. What if, for example, TimedMediaHandler wants a slot of its own for subtitles, with its own "combined output" code? Anomie (talk) 20:32, 26 February 2018 (UTC)
 * Since a completely generic combining mechanism seems insufficiently flexible (or would have to be extremely complex), and it's unclear how multiple extensions would cooperate on this, the present idea is to have a singe extension control the rendering of a page. I could also imagine us allowing a single extension to do complex composition, while allowing other extensions to add stuff at the top or bottom.
 * Anyway, what's the alternative? Am I understanding correctly that you want to use wikitext in the main slot as the only composition mechanism? -- Daniel Kinzler (WMDE) (talk) 18:39, 28 February 2018 (UTC)
 * Specifically in the case of TimedMediaHandler, it'll probably want to customize the MediaTransformOutput rather than the ParserOutput as the subtitles relate to the image and not the description wikitext. But in general, yeah, the ownership of composition seems tricky. One approach is that content handlers have access to what role the content is in (seems like a good idea in general) and can request to be included in the view. (So by default only the main content is visible but other slots can opt in.) Each of the visible slots could have access to all others and do its own composition then. --Tgr (WMF) (talk) 10:19, 1 March 2018 (UTC)
 * My initial thinking was also that this should be controlled by the ContentHandler. But then I realized that this is not a good separation of concerns, and gets in the way of extensibility. E.g. for template documentation, both the main and the doc slot are wikitext. Why should the handler for a content model know about the different purposes that kind of content can be used for? And how could extensions add to that? If it's the main slot's ContentHandler that does the composing (as I originally envisioned), how will the wikitext handler on File pages know how to deal with MediaInfo?
 * It seems to make much more sense to me to bind this behavior (a PageOutputComposer) to a "page type" - which, for now, is given by the namespace. But yes, if multiple extensions want to inject things into the File page's layout, some kind of generic handling needs to be possible. Perhaps the PageOutputComposer could have a list of "other slots" to add at the bottom, to which extensions can add.
 * Note btw that all slots need to be accessible via a single-slot view, to ensure no content becomes inaccessible when configuration changes or strange things happen due to undeletion or import. -- Daniel Kinzler (WMDE) (talk) 14:09, 1 March 2018 (UTC)
 * Note that for output in the user language, the ParserCache needs to be split (which is already the case for File pages on Commons)

Multi-Slot View (Anomie's alternative)

 * The default view will show the content of the main slot, as it does now for the non-MCR case.
 * The main slot can explicitly transclude content from other slots, via parser functions or tags, as can happen now. This probably won't generate a ParserOutput for that slot, although that's a possibility if the implementation of the function/tag finds that useful.
 * Content from other slots might also be implicitly "transcluded", much like how Extension:Cite does now when no is present in the wikitext. This need not generate a ParserOutput either, e.g. a "metadata" slot might just call addCategory, addDisplayTitle, and so on to add to the ParserOutput from the main slot. We could locate the code for implicit transclusion in hooks (as Extension:Cite's implicit  does now), or we could add it to Content or ContentHandler to avoid having to use hooks.
 * So, the main implication is that the default view will always be the main slot only. The only way to show things from other slots in the default view would be to explicitly access it from the main slot's wikitext (or other content), or by using hooks to do something very similar to what I am proposing with the combined PO.
 * Since pretty much all the use cases that have been discussed for MCR (Requests_for_comment/Multi-Content_Revisions) assume that the content of all slots is visible per default, it like a good idea to establish a standard mechanism for achieving this easily. I don't see how this can be done in Content or ContentHandler, I'd do it in a separate "PageOutputComposer" or "PageTypeHandler", since the composition logic doesn't have anything to do with the content model. Point in case, for e.g. template documentation, all slots would be wikitext. -- Daniel Kinzler (WMDE) (talk) 22:36, 28 February 2018 (UTC)
 * Of the use cases described there, structured media info, gadget styles and file history should be visible by default; infobox data, (probably) the page metadata, templatedata, templatestyles, (probably) video subtitles, assessments, focus area, CentralNotice bits, ProofreadPage and blame maps shouldn't. I could see categories and template/module docs go either way - get their own "view slot" or just get pulled in by the main content. (Arguably they should work with arbitrary types of main content so maybe getting their own slot makes more sense.) Some use cases discussed elsewhere were Wikidata description overrides and page annotations, which also should not be visible. So only displaying the main content seems like a reasonable default. Having something like  seems like an OK way to allow easy opt-in, although the content handler would have to know the role for that (and maybe at that point it makes more sense to have some kind of role handler?). As for PageTypeHandler, how would it differ from the content handler of the main slot? We have no such thing as page type. --Tgr (WMF) (talk) 11:13, 1 March 2018 (UTC)
 * I actualyl disagree on may of the things you say should be invisible (why should infoboxes be invisible? or templatedata? or assessments? I believe they should all be visible in some way somewhere on the page - they just shouldn't have their own section). But that's not what needs to be decided here.
 * The question is whether code should assume that "the page content" is the content of the main slot, and other stuff should only be shown if the main slot's content, or the main slot's ContentHandler, or something hooking into that, wants to show it. That means that in code, there is no distinction between "main slot content" and "all slot's content". Even ignoring the HTML composition, this poses problems for tracking secondary data of non-main slots: it would only be tracked if explicitly looped through by the main slot.
 * I would much prefer a mechanism that allows us to always track all secondary info from all slots, as defined by their ContentHandlers. I would also prefer to not bind the decision what is shown to the content model of the main slot. I also don't see how it belongs into the content handler of the "extra" slot. As I said, both can be wikitext, and why should the wikitext handler know what loa module documentation is, and whether it should be visible per default? That's a decision that belongs to "module pages", in my mind.
 * But again - besides when and where and how to compose HTML, the major issue is how to track secondary info, and when and where to determin what DataUpdates need to be run to track it. If we don't want to change much about how DataUpdates work, we'll have to call Content::getSecondaryDataUpdates for each slot, for the ParserOutput of that slot. I'd really like to avoid having to fundamentally change that. -- Daniel Kinzler (WMDE) (talk) 14:25, 1 March 2018 (UTC)
 * On the other hand, if the "page's content", aka the "standard view", is the same as the "all slots view", then there's no way for a slot to show more helpful HTML output when viewed non-standardly. For example, when viewing a template that has a TemplateStyles slot you'd either never be able to see the pretty-printed CSS or you'd have to always see it on the page. Or for article assessments, there'd be no way short of opening the editor to get a more explicit list of the assessments than whatever icons the assessments might add to the default view of the page (and, at least on enwiki, assessments other than "good article" and "featured article" only belong on the talk page with no indication on the article itself). I find it odd that you want to add a whole inflexible "page controller" infrastructure and a model that doesn't allow a slot to not be displayed by default to avoid any changes to how  might work for non-main slots. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * I think we are getting to the core of the misunderstanding here.
 * The idea is not that the standard view has to show the HTML of all slots. And the composition logic for the standard view may use HTML from each slot's own PO, but it could also render the content of other slots in some other way, or ask the slot's Content object to render in some other way for use in the standard view. The composition logic can just as well ignore some slots.
 * I think whatever the standard view is should be in the ParserCache. This is the easiest thing to do, and it matches the expectations of code that reads from the PC. However, having the POs of each slot, or rather, the slots used to generate the HTML of the standard view, "nested" in a combined PO is just an idea for optimized re-rendering. Nothing forces us to do that.
 * But we do need to generate the POs for all slots when saving an edit, so we know what to put into the links tables. But they don't have to be used to generate the main view. This is actually the reason I put the list of per-slot POs and also the combined PO into the PreparedEdit. The need and opportunity for this split occurred to me while working on this patch. Previously, I thought it would be necessary to feed the secondary tables from the combined PO. This is not the case - in fact, it wouldn't work without fundamental changes to getSecondaryDataUpdates. Perhaps this is what caused the confusion.
 * We could try to only generate "blind" POs, with no HTML, but it's tricky to know in advance for which slots we won't later need to HTML. So I'd leave that optimization for later. -- Daniel Kinzler (WMDE) (talk) 11:48, 2 March 2018 (UTC)
 * No, we don't need POs for every slot to handle links tables. All we need is that any necessary links wind up in a PO that gets passed into LinksUpdate. That can be the "combined" PO, which as I've already said need not actually be generated by combining per-slot POs. Anomie (talk) 14:34, 2 March 2018 (UTC)
 * I don't see how this can be done in Content or ContentHandler When MediaWiki is doing the implicit transclusion step, instead of (or in addition to) calling a hook it could do something like . The actual implementation may, of course, be more complicated (e.g. consideration of ordering), but that's the basic idea for letting the Content or ContentHandler do implicit transclusion in my model. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * It seems odd, logically, to bing this to the ContentHandler. The ContentHandler knows about a specific kind of content, it does not know about the roles this content may have on a page.
 * I initially also though this would be the easiest approach. But now it seems clear to me that injecting handling for concepts like template documentation or structured media info into the WikitextContentHandler is backwards and unnecessary.
 * It seems to me that attaching the composition logic to page types gives us flexibility, it doesn't take any ways. -- Daniel Kinzler (WMDE) (talk) 11:48, 2 March 2018 (UTC)
 * As soon as you say "into the WikitextContentHandler", you should know that you've missed the point. A documentation slot could be a "DocWikitextContentHandler" subclass of WikitextContentHandler. Or it could be done with hooks. Or perhaps we could have slots register handlers that aren't ContentHandlers, which would still be better than a one-per-page handler. Anomie (talk) 14:34, 2 March 2018 (UTC)
 * It should also be possible to view an HTML rendering of a non-main slot. This would generate a ParserOutput for the slot.
 * The parser cache will likely need to be extended to be able to cache non-main slot ParserOutput objects, both for the non-main viewing and for use by potential implicit transclusion. Whether we cache only the main slot's pre-implicit-transclusion ParserOutput (and run implicit transclusion as a post-cache transformation), only the post-implicit-transclusion ParserOutput (and thus re-parse the main slot whenever any slot changes), or both is yet to be determined.
 * I was trying to avoid that. Partially just to avoid additional work, but also because code reading from the ParserCache expects the PO in there to be "the page output". Which it wouldn't be if we only put the main slot there. Basically, my thinking is that all slots contribute to the page's output. Your approach seems to be that the main slot is the page output, but there may be other page output, which may or may not be tagged onto the main output. -- Daniel Kinzler (WMDE) (talk) 22:36, 28 February 2018 (UTC)
 * Is it really so much extra work to add an optional $slot parameter or saveForSlot/getForSlot methods if a slot wants to put its own ParserOutput into the cache? That seems like a lot less work than adding a new "page controller" concept. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * Anomie, I just realized: your pre-implicit-transclusion output is my "main slot" output. Your post-implicit-transclusion output is my "combined" output. We are not disagreeing here. It's the same thing. We need both somewhere - and working on the initial WikiPage refactoring, I found that things become a lot easier if both are available from the PreparedEdit (I was trying to avoid that, but kept running into trouble). We can keep talking about how the code that does the "implicit transclusion" aka "output composition" lives - but that discussion does not need to block the current patch. For that patch, the only thing that matters is that we need a way to a) get a PO for each slot and b) get a PO with implicit transclusion (aka output composition) applied. And I did not find a way to avoid having both in PreparedEdit -- Daniel Kinzler (WMDE) (talk) 13:49, 1 March 2018 (UTC)
 * We're disagreeing on some of the details. You want to parse every slot and have a one-per-page class to combine the slots' ParserOutputs, which means that you're designing everything around actually creating and passing around all those POs. I want to begin with the one "main" ParserOutput and let code for non-main slots add to it using a hook model, without creating additional POs if it's not actually needed for the slot's use case and without a one-per-page combiner. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * As I pointed out above, the other POs are always needed, at least temporarily. The baseline patch passes them around only as much as needed to feed the links tables.
 * I'd like to reduce the usage of hooks, but some kind off extension point / injection will be needed for whatever code composes the HTML for the standard view. I see no reason for this code to live in a ContentHandler, though. -- Daniel Kinzler (WMDE) (talk) 11:48, 2 March 2018 (UTC)
 * As I pointed out above, the other POs are not always needed. We're having this discussion because I disagree with how you implemented your "baseline" patch, so appealing to it is a bogus argument. Anomie (talk) 14:34, 2 March 2018 (UTC)
 * Access to other slots of the revision will proceed by loading the revision, normally by using Parser::fetchCurrentRevisionOfTitle. If a Parser instance isn't available, ParserOptions::getCurrentRevisionCallback could be used more directly or via a similar helper.
 * This could work, but we'll want to be able to use this before a Revision has been saved. In my mind it makes more sense to have a way to pas the ParserOptions for other slots - which may or may not come from the revision. -- Daniel Kinzler (WMDE) (talk) 13:49, 1 March 2018 (UTC)
 * Even if the RevisionRecord isn't saved yet, we should be able to construct one. It may be more complicated for MCR than what ParserOptions::setupFakeRevision does currently, but that's the general idea. That also has the benefit of working well with existing code like TemplateSandbox. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * If that'S the easiest way, then let's do that. -- Daniel Kinzler (WMDE) (talk) 11:48, 2 March 2018 (UTC)

The SDoC case could work like Extension:Cite's : if the wikitext includes a tag to say "insert the structured data here" it will do so, and if not it'll append the structured output to the main slot's wikitext output.
 * Appending to the wikitext will not work, it would need to append full HTML, plus registering the necessary JS modules, CSS modules, tracking info for the links table, etc. This sounds a lot like the "combined" PO I was suggesting, but it's unclear to me how the Wikibase content can register its DataUpdates. Probably by hooking in again at some later point. Basically, this sounds like an emulation of my proposal using hooks instead of defining a mechanism for composing a PO from multiple slots.
 * Overall, this seems to be more work: we have to modify the parser cache, introduce new hooks, provide ways to access all kinds of content from withing wikitext, or use several hooks for every slot so it can attach itself to the main PO. I'd rather have a mechanism that just defines how the combine PO is composed, with a per-page-type handler for combining the HTML in the desired way. -- Daniel Kinzler (WMDE) (talk) 22:22, 28 February 2018 (UTC)
 * "to the main slot's wikitext" was not an accurate statement, I meant "to the main slot's output". A ParserOutput doesn't even have wikitext to append to. And yes, in the case of SDC, it would be a lot like the "combined" PO you're suggesting. Your DataUpdates can probably be registered in the same way they are now, by having MediaWiki call Content::getSecondaryDataUpdates on the content of each slot. But it's not "an emulation of [your] proposal". Your proposal wants to generate individual PO objects then combine them using a one-per-page combiner class. My proposal wants to allow multiple Content objects to contribute to a single PO without a one-per-page combiner, which need not proceed by generating a PO for each one. I don't think my proposal is more work, but it's not "I get to write entirely new code instead of refactoring old code" like your proposal is. We may have to make a minor extension to the parser cache, "ways to access all kinds of content from within wikitext" would have to happen anyway unless you want to be even more inflexible and forbid explicit transclusion, and it'll much more likely be one hook than several. Anomie (talk) 17:25, 1 March 2018 (UTC)


 * I think there are orthogonal issues here:
 * Pull model (only the main content is parsed; it can initiate the parsing of other slots if it desires) vs. push model (all slots are are parsed and then the ParserOutput objects merged somehow). The latter seems easier to coordinate.
 * PO object hierarchy: should the non-main content POs be merged into / provided to the main content PO, or should all the content POs combined into a new revision-level PO? I guess that depends on whether we expect that typically the main content type determines how combining should work, or it's something that would typically be controlled by an extension that registers itself in some content-type-unrelated way. I can't really think of any counterexamples to the first option.
 * --Tgr (WMF) (talk) 11:13, 1 March 2018 (UTC)
 * SDC is a counter-example. The main slot is wikitext. How or why should it know or care about MediaInfo? The wikitext will have access to structured data via Lua modules, but the wikitext content model doesn't know about that. And this has nothing to do with exposing the full structured data as HTML along with the rendered wikitext description. -- Daniel Kinzler (WMDE) (talk) 14:28, 1 March 2018 (UTC)
 * Neither of our proposals are having the wikitext ContentHandler know or care about MediaInfo, but IMO yours is closer to that model. You want to add a "page model" that forces the use of the SDC content-combiner. I want combining to proceed based on the slots that are present: the SDC code is called when the SDC slot exists to add that slot's content to a ParserOutput that may or may not have been generated by a wikitext main slot, and other extensions' code is called for their slots if those also exist. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * "Pull model" versus "push model" is not an accurate characterization here. My proposal has both pull (explicit transclusion) and push (implicit transclusion). Daniel's does too unless he's forbidding explicit transclusion. The differences are in how exactly the pushing works: generate POs for each Content and have a one-per-page combiner operate on those POs, or have each Content push output into a single PO without a one-per-page combiner to coordinate it. The former (Daniel's) is simpler but less flexible. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * In your model, how does WikitextContent used as template documentation know where and how to "push" itself into the main PO? -- Daniel Kinzler (WMDE) (talk) 11:48, 2 March 2018 (UTC)
 * To avoid duplication, I'll just mention here that I answered that above. Anomie (talk) 14:34, 2 March 2018 (UTC)

Multi-Slot Editing
Model use case: Atomic editing of a Lua module and its documentation. Single-slot editing should also be possible.


 * Allow EditPage to show one editing area (for text based content: one text area) for each slot. Also provide a single-slot editing mode.
 * Allow previews to be rendered for all slots or individual slots.
 * When parsing/rendering after save, re-use the cached rendering of unchanged slots, instead of regenerating it needlessly. However, if the rendering of the unchanged slot depends on a slot that was touched, it needs to be re-rendered anyway.

This implies that ApiStashEdit, ApiParse, and ApiEditPage all accept input for multiple slots at once.


 * IMO trying to figure out the edit view based solely on architecture requirements and only incuding designers when it's fully implemented will end badly. Just throwing out a bunch of text boxes under each other is probably not acceptable UX. (It will be needed as a non-JS fallback as there is little else to do int that case, but usually some mechanism will be required to decide which component gets to control the rendering. Maybe some kind of tabbed view would make sense for handling multiple large edit boxes. TimedMediaHandler might have 200 slots for subtitle translations that it wants to handle in a single view block, without interfering how the rest of the content is rendered. VisualEditor takes over the whole content area, it might want to force the non-main components into dialog boxes. What would be a reasonable interface on a mobile screen? etc.) It would be good to ask for a set of mock designs based on the use cases collected so far, just to have some ideas to base the architecture planning on. --Tgr (WMF) (talk) 07:25, 1 March 2018 (UTC)
 * You are right that we shouldn't discuss UI specific here. The point is that we need to support the case of the updated content of multiple slots coming in via a single POST request, and that we may want to optimize for the case where one slot changed, and another did not. If we don't need this optimization, we don't need a "hierarchical" ParserOutput. Then it is sufficient to cache the composed HTML, and regenerate the whole thing by re-parsing all slots on edit. -- Daniel Kinzler (WMDE) (talk) 14:33, 1 March 2018 (UTC)
 * We might avoid the need for that optimization by not actually parsing every slot unconditionally, instead letting the individual slots decide whether and what kind of caching is needed. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * On the other hand, getting designers involved too early is also going to end badly, and having Daniel and/or I (rather than developers more familiar with front-end coding) being the ones to implement the designers' designs might also be a bad end. If you want to take on the responsibility for redoing the UI of EditPage, as far as I'm concerned you're welcome to it. Otherwise my plan is to put together a MVP (which might not even get merged) for the textarea-based editor and let others who're better qualified handle VE and improvements to the textarea-based editor. I might make a tabbed interface, or I might simply show one slot at a time and force the user to submit to change slots. And I hope TimedMediaHandler doesn't actually add a slot per language. Anomie (talk) 17:25, 1 March 2018 (UTC)

Proposed Changes to ParserOutput

 * Each ParserOutput can record which slots it depends on
 * The combined canonical PO has to return aggregated information of all slots from all getters
 * The combined canonical PO should not contain duplicate information, since it will be serialized and stored in the parser cache, so size matters.

Option 1, compose output just in time:
 * The combined ParserOutput for all slots would have to
 * know the ParserOutput objects for each slot
 * getters aggregate info from all the POs on the fly.
 * know how to combine the HTML from all slot's ParserOutputs into a single output (or return null from getText), by substituting placeholders in an HTML "template". Note that this template may be defined or modified by an extension, and may depend on page type or namespace or some other property of the page.

Options 2, compose output right away:
 * The combined parser output gets fed with the aggregated content of the POs.
 * The code that constructs the combined PO also constructs the combined HTML right away.
 * This means that during saving, the POs of unchanged slots cannot be re-used.

Note that option 1 and 2 offer largely the same interface to calling code. The only code that needs to be aware of this distinction is the PO itself, and the code that constructs it.

This means that we can start with option 2, and change to option 1 later when need arises, for optimizing the rendering during save, or for other purposes. E.g. AbuseFilter may want to apply different rules to different slots, so it would have to process the POs of the individual slots separately.


 * Since all this depends on your model of how things work that I disagree with, I find all this unnecessary as well. Anomie (talk) 20:32, 26 February 2018 (UTC)
 * If I understand correctly, you want to "somehow" merge all info from the extra slots into the ParserOutput of the main slot. That seems very close to my proposal, the only difference being whether we conceptually have a "combined" PO that is different from the main slot's PO or not. I think it would be useful for this discussion if you could state your proposal below, and explain how it meets the requirements above. If you disagree with the requirements, please explain how and why. -- Daniel Kinzler (WMDE) (talk) 18:27, 28 February 2018 (UTC)
 * That's largely correct, although we could still have a "combined" PO (post-implicit-transclusion) that is different from the main slot's PO (pre-implicit-transclusion) in my proposal. The real differences are whether we force every slot to generate a PO to make that "combined" PO and how exactly the combining happens. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * Actually, for the "combine output using hooks" version of your proposal, the need/opportunity for optimization is the same. The difference is then just whether we have separate keys for each slot in the ParserOutput. I like separate keys in principle, but see little practical advantage, and I expect sticking with a single key is going to be a lot simpler. -- Daniel Kinzler (WMDE) (talk) 23:05, 28 February 2018 (UTC)
 * You're still hung up on the idea of always generating a PO for every slot. That need not happen to generate the "combined" PO, even if it is needed for a non-standard view of that slot. And even if a PO is created for a slot, we don't necessarily have to cache it if we know it's always going to be cheap to regenerate. Anomie (talk) 17:25, 1 March 2018 (UTC)
 * I'm hung up on the idea of tracking page links and image links and template usage etc for all slots, even if they are not visible in the standard view, yes. It seems to be that this is what we disagree on. Let's get input on this question from other developers and product folks. -- Daniel Kinzler (WMDE) (talk) 11:53, 2 March 2018 (UTC)

Page Type Considerations
I have said above that I think the "composition logic" for combining HTML from different slots should be bound to the concept of "page type", e.g. "file page" or "article page" or "template page", etc.

This isn't absolutely necessary, though. My main point is that this logic should not be bound to a content model. What could be an option is to have "combining logic" for each slot role. However, I like the "page type" approach better, for two reasons:

1) If we leave the combining to the respective slots, all they can do it append their HTML above or below what's already there. More complex composition isn't possible.

2) We will need "page type handlers" of sorts anyway - in fact, we already have them, in the form of FilePage/WikiFilePage and CategoryPage/WikiCategoryPage. If we want to refactor WikiPage and Article, these need to be replaced by some kind of "file type handler", with extension being able to add their own (just like they now do via the WikiPageFactory and ArticleFromTitle hooks). These handlers would have the responsibility to provide the appropriate action handers, diff view, edit page, redirection logic, purging logic, etc. All these functions will have to involve all the slots. It seems natural to also place the content composition there.

In any case, for factoring PageUpdater out of WikiPage, none of this needs to be decided. The first iteration can just use the main slot for the "composed" standard view. -- Daniel Kinzler (WMDE) (talk) 13:37, 2 March 2018 (UTC)
 * Having "combining logic" for each slot role is one of the two main features of my proposal. Your #1 is entirely false. There's nothing that can be done in a per-page combiner that can't be done by a per-slot combiner unless you're doing crazy things with multiple unrelated slots that the slots themselves know nothing about or you're "combining" the one main slot with no non-main slots in some weird way. Your #2 is also false. Yes, we have FilePage, but we could store reference to the file as a slot and get almost everything there that way (the missing bits are, potentially, some details of the show-Commons-locally functionality). CategoryPage goes beyond what can sanely be done with a per-page MCR combiner anyway, it's closer to being an implicit special page. The problem with PageUpdater is that you're basing decisions like "we need a PO for every slot" on things that need deciding here. Anomie (talk) 14:34, 2 March 2018 (UTC)