Subramanya Sastry has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/329352 )
Change subject: Cleanup in TemplateHandler ...................................................................... Cleanup in TemplateHandler * Removed stale code and refactored control flow to make the logic a bit more clearer. There was (and still is) a fair bit of cruft here. Change-Id: I0a7e3a593eca93a0e4fd9fbac6cfde803ebd524c --- M lib/wt2html/tt/TemplateHandler.js M tests/parserTests.txt 2 files changed, 89 insertions(+), 94 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid refs/changes/52/329352/1 diff --git a/lib/wt2html/tt/TemplateHandler.js b/lib/wt2html/tt/TemplateHandler.js index e11ad4f..e42c8df 100644 --- a/lib/wt2html/tt/TemplateHandler.js +++ b/lib/wt2html/tt/TemplateHandler.js @@ -63,51 +63,50 @@ * processes the template. */ TemplateHandler.prototype.onTemplate = function(token, frame, cb) { + var atm; var env = this.env; - - // magic word variables can be mistaken for templates - try { - var magicWord = this.checkForMagicWordVariable(token); - if (magicWord) { - cb({ tokens: Array.isArray(magicWord) ? magicWord : [magicWord] }); - return; - } - } catch (e) { - env.log("error", "Exception checking magic word for token: ", token); - } - + var text = token.dataAttribs.src; var state = { token: token, wrapperType: 'mw:Transclusion', wrappedObjectId: env.newObjectId(), + recordArgDict: this.options.wrapTemplates, srcCB: this._startTokenPipeline, - }; - - if (this.options.wrapTemplates) { - state.recordArgDict = true; // Uncomment to use DOM-based template expansion // TODO gwicke: Determine when to use this! // - Collect stats per template and classify templates into - // balanced/unbalanced ones based on it + // balanced/unbalanced ones based on it // - Always force nesting for new templates inserted by the VE - // state.srcCB = this._startDocumentPipeline; - + // // Default to 'safe' token-based template encapsulation for now. + // + // srcCB: this._startDocumentPipeline + }; + + var tgt = this.resolveTemplateTarget(state, token.attribs[0].k); + if (tgt && (tgt.isPF || tgt.isMagicWord)) { + // magic word variables can be mistaken for templates + try { + var magicWord = this.checkForMagicWordVariable(token); + if (magicWord) { + cb({ tokens: Array.isArray(magicWord) ? magicWord : [magicWord] }); + return; + } + } catch (e) { + env.log("error", "Exception ", e, " checking magic word for token: ", token); + } } - var text = token.dataAttribs.src; - var tgt = this.resolveTemplateTarget(state, token.attribs[0].k); - var accumReceiveToksFromSibling; - var accumReceiveToksFromChild; - - // console.warn("\ttgt", tgt); if (this.options.wrapTemplates && tgt === null) { // Target contains tags, convert template braces and pipes back into text // Re-join attribute tokens with '=' and '|' this.convertAttribsToString(state, token.attribs, cb); return; } + + var accumReceiveToksFromSibling; + var accumReceiveToksFromChild; if (env.conf.parsoid.usePHPPreProcessor) { if (this.options.wrapTemplates) { @@ -193,15 +192,17 @@ // TokenAccumulator and can return the expansion directly accumReceiveToksFromSibling = cb; } + + accumReceiveToksFromSibling({tokens: [], async: true}); + // expand argument keys, with callback set to next processing step // XXX: would likely be faster to do this in a tight loop here - var atm = new AttributeTransformManager( + atm = new AttributeTransformManager( this.manager, {wrapTemplates: false, inTemplate: true}, this._expandTemplate.bind(this, state, frame, tgt, accumReceiveToksFromSibling) ); - accumReceiveToksFromSibling({tokens: [], async: true}); atm.process(token.attribs); } }; @@ -242,6 +243,7 @@ // 2. String with entities: {{DEFAULTSORT:"foo"bar}} // 3. Templated key: {{DEFAULTSORT:{{foo}}bar}} + var env = this.manager.env; var property, key, propAndKey, keyToks; var magicWord = tplToken.attribs[0].k; @@ -281,11 +283,11 @@ } property = property.trim(); - var name = this.manager.env.conf.wiki.magicWords[property]; + var name = env.conf.wiki.magicWords[property]; // try without the colon if (!name) { - name = this.manager.env.conf.wiki.magicWords[property.slice(0, -1)]; + name = env.conf.wiki.magicWords[property.slice(0, -1)]; } // special case for {{!}} magic word @@ -301,7 +303,7 @@ var state = { token: tplToken, wrapperType: "mw:Transclusion", - wrappedObjectId: this.manager.env.newObjectId(), + wrappedObjectId: env.newObjectId(), }; this.resolveTemplateTarget(state, "!"); @@ -362,7 +364,7 @@ }; TemplateHandler.prototype.resolveTemplateTarget = function(state, targetToks) { - function resolvabilityInfo(tokens) { + function toString(tokens) { var maybeTarget = Util.tokensToString(tokens, true); if (Array.isArray(maybeTarget)) { var allString = true; @@ -382,104 +384,97 @@ && ntt.name !== 'templatearg') { // We are okay with empty (comment-only) lines, // {{..}} and {{{..}}} in template targets. - return { isStr: false }; + return null; } } else if (ntt.constructor === CommentTk) { // Ignore comments as well allString = false; } else if (ntt.constructor === TagTk || ntt.constructor === EndTagTk) { - return { isStr: false }; + return null; } } - return { isStr: true, isSimpleTgt: allString, newTarget: newTarget }; + return newTarget; } else { - return { isStr: true, isSimpleTgt: true }; + return maybeTarget; } } var env = this.manager.env; + var wiki = env.conf.wiki; + var isTemplate = true; + var target = toString(targetToks); - // Convert the target to a string while stripping all non-text tokens - var target = Util.tokensToString(targetToks).trim(); + if (!target) { + // Retry with a looser attempt to convert tokens to a string. + // This lenience only applies to parser functions. + isTemplate = false; + target = Util.tokensToString(targetToks); + } - // strip subst for now. - target = target.replace(/^(safe)?subst:/, ''); - - // Check if we have a parser function. - // - // Unalias to canonical form and look in config.functionHooks + // Without a trim(), we get bug T147742 because + // the prefix === target check below fails! + target = target.trim().replace(/^(safe)?subst:/, ''); var pieces = target.split(':'); var prefix = pieces[0].trim(); var lowerPrefix = prefix.toLowerCase(); - var magicWordAlias = env.conf.wiki.magicWords[prefix] || env.conf.wiki.magicWords[lowerPrefix]; + var isSpecialMagicWord = (target === '!'); + + // Check if we have a parser function. + // Unalias to canonical form and look in config.functionHooks + var magicWordAlias = wiki.magicWords[prefix] || wiki.magicWords[lowerPrefix]; var translatedPrefix = magicWordAlias || lowerPrefix || ''; // The check for pieces.length > 1 is require to distinguish between // {{lc:FOO}} and {{lc|FOO}}. The latter is a template transclusion // even though the target (=lc) matches a registered parser-function name. if ((magicWordAlias && this.parserFunctions && this.parserFunctions['pf_' + magicWordAlias]) || - (pieces.length > 1 && (translatedPrefix[0] === '#' || env.conf.wiki.functionHooks.has(translatedPrefix)))) { + (pieces.length > 1 && (translatedPrefix[0] === '#' || wiki.functionHooks.has(translatedPrefix)))) { state.parserFunctionName = translatedPrefix; return { isPF: true, + magicWord: target, prefix: prefix, target: 'pf_' + translatedPrefix, pfArg: target.substr(prefix.length + 1), }; } - // We are dealing with a real template, not a parser function. - // Apply more stringent standards for template targets. - var tgtInfo = resolvabilityInfo(targetToks); - if (tgtInfo.isStr && !tgtInfo.isSimpleTgt) { - // resolvabilityInfo found a new target based on the target tokens. This - // happens when the target contains special characters, specially quotes. - // For an example, look at T96090. - - // Without a trim(), we get bug T147742 because - // the prefix === target check below fails! - target = tgtInfo.newTarget.trim(); - pieces = target.split(':'); - prefix = pieces[0].trim(); - lowerPrefix = prefix.toLowerCase(); - } - - if (tgtInfo.isStr) { - // FIXME: resolveTitle adds namespace prefix when it resolves - // fragments and relative titles. Maybe we should add a flag to - // have it do in all cases. - if (prefix === target && !/^[#\/\.]/.test(target)) { - var namespaceId = env.conf.wiki.canonicalNamespaces.template; - target = env.conf.wiki.namespaceNames[namespaceId] + ':' + target; - } - - // Resolve a possibly relative link and - // normalize the target before template processing. - var title; - try { - title = env.resolveTitle(target); - } catch (e) { - // Invalid template target! - return null; - } - - // Entities in transclusions aren't decoded in the PHP parser - // So, treat the title as a url-decoded string! - title = env.makeTitleFromURLDecodedStr(title, undefined, true); - if (!title) { - // Invalid template target! - return null; - } - - // data-mw.target.href should be a url - state.resolvedTemplateTarget = env.makeLink(title); - - return { isPF: false, target: title.getPrefixedDBKey() }; - } else { + if (!isTemplate) { return null; } + // FIXME: resolveTitle adds namespace prefix when it resolves + // fragments and relative titles. Maybe we should add a flag to + // have it do in all cases. + if (prefix === target && !/^[#\/\.]/.test(target)) { + var namespaceId = wiki.canonicalNamespaces.template; + target = wiki.namespaceNames[namespaceId] + ':' + target; + } + + // Resolve a possibly relative link and + // normalize the target before template processing. + var title; + try { + title = env.resolveTitle(target); + } catch (e) { + // Invalid template target! + return null; + } + + // Entities in transclusions aren't decoded in the PHP parser + // So, treat the title as a url-decoded string! + title = env.makeTitleFromURLDecodedStr(title, undefined, true); + if (!title) { + // Invalid template target! + return null; + } + + // data-mw.target.href should be a url + state.resolvedTemplateTarget = env.makeLink(title); + + return { isPF: false, isMagicWord: isSpecialMagicWord, target: title.getPrefixedDBKey() }; + }; TemplateHandler.prototype.convertAttribsToString = function(state, attribs, cb) { diff --git a/tests/parserTests.txt b/tests/parserTests.txt index 1205f53..c5dd865 100644 --- a/tests/parserTests.txt +++ b/tests/parserTests.txt @@ -10325,7 +10325,7 @@ !! wikitext {{DISPLAYTITLE:''{{PAGENAME}}''}} !! html/parsoid -<meta property="mw:PageProp/displaytitle" content="Main Page" about="#mwt2" typeof="mw:ExpandedAttrs" data-parsoid='{"src":"{{DISPLAYTITLE:''{{PAGENAME}}''}}"}' data-mw='{"attribs":[[{"txt":"content"},{"html":"<i data-parsoid='{\"dsr\":[15,31,2,2]}'><span about=\"#mwt1\" typeof=\"mw:Transclusion\" data-parsoid='{\"pi\":[[]],\"dsr\":[17,29,null,null]}' data-mw='{\"parts\":[{\"template\":{\"target\":{\"wt\":\"PAGENAME\",\"function\":\"pagename\"},\"params\":{},\"i\":0}}]}'>Main Page</span></i>"}]]}'/> +<meta property="mw:PageProp/displaytitle" content="Main Page" about="#mwt3" typeof="mw:ExpandedAttrs" data-parsoid='{"src":"{{DISPLAYTITLE:''{{PAGENAME}}''}}"}' data-mw='{"attribs":[[{"txt":"content"},{"html":"<i data-parsoid='{\"dsr\":[15,31,2,2]}'><span about=\"#mwt2\" typeof=\"mw:Transclusion\" data-parsoid='{\"pi\":[[]],\"dsr\":[17,29,null,null]}' data-mw='{\"parts\":[{\"template\":{\"target\":{\"wt\":\"PAGENAME\",\"function\":\"pagename\"},\"params\":{},\"i\":0}}]}'>Main Page</span></i>"}]]}'/> !! end !! test -- To view, visit https://gerrit.wikimedia.org/r/329352 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0a7e3a593eca93a0e4fd9fbac6cfde803ebd524c 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