Subramanya Sastry has uploaded a new change for review.
https://gerrit.wikimedia.org/r/204771
Change subject: T93368: Fix crashers seen in production
......................................................................
T93368: Fix crashers seen in production
* As it turns out, wrapTemplates is always true in the
AttributeExpander and it made for a poor check for when
we can rely on tsr being present on a token. So, we had
a mismatch where the template-expander removes tsr from
its tokens and AttributeExpander none the wiser with its
useless wrapTemplates check.
* In combination with wikitext markup errors, and possibly
some weirdness in the PHP expandtemplates api endpoint
which sends back unexpanded transclusions, this is what
introduced the crasher scenario.
* Now fixed by checking for presence of tsr now. Verified
that all pages in that bug report are fixed. I have also
fixed markup on one of the pages, but didn't investigate
the source of (likely) markup errors on the other two pages.
Change-Id: If2a774a0884a7c131c418c8d6c7e122807560005
---
M lib/ext.core.AttributeExpander.js
1 file changed, 20 insertions(+), 4 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid
refs/changes/71/204771/1
diff --git a/lib/ext.core.AttributeExpander.js
b/lib/ext.core.AttributeExpander.js
index f8fb63e..9c2ad5d 100644
--- a/lib/ext.core.AttributeExpander.js
+++ b/lib/ext.core.AttributeExpander.js
@@ -56,6 +56,11 @@
}
function splitTokens(env, token, nlTkPos, tokens, wrapTemplates) {
+ // FIXME: It is insufficient to rely merely on wrapTemplates
+ // because right now, it is always true. Since tsr values are
+ // stripped from template tokens, we use that as a proxy.
+ wrapTemplates = wrapTemplates && token.dataAttribs.tsr;
+
var buf = [], postNLBuf, startMeta, metaTokens;
// Split the token array around the first newline token.
@@ -205,11 +210,22 @@
* @private
*/
AttributeExpander.prototype._returnAttributes = function( token, cb,
expandedAttrs ) {
- var env = this.manager.env;
+ // SSS FIXME: This is mostly useless.
+ //
+ // wrapTemplates will always be true for all tokens from the top-level
+ // as well as tokens coming from template expansions because template
+ // content only goes through the PEG in a separate pipeline and the
+ // resulting tokens are merged back into the main top-level pipeline
+ // which has wrapTemplates set to true. To see this, look at the
+ // default pipeline type in
ext.core.TemplateHandler.js:_startTokenPipeline
+ // and check the components of that pipeline type in mediawiki.parser.js
+ //
+ // Currently, this doesn't matter a whole lot since templates are
currently
+ // fully expanded with the PHP preprocessor and we encounter
transclusions
+ // only from the top-level. However, when T93368 scenarios happen (or
when
+ // we are in the native parsoid pipeline), this could be a more serious
issue.
var wrapTemplates = this.options.wrapTemplates;
-
- env.dp( 'AttributeExpander._returnAttributes: ', expandedAttrs );
-
+ var env = this.manager.env;
var modified = false;
var metaTokens = [];
var postNLToks = [];
--
To view, visit https://gerrit.wikimedia.org/r/204771
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If2a774a0884a7c131c418c8d6c7e122807560005
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <ssas...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
