Fix SectionOnPage.ejs when page title includes angle brackets #320
Conversation
|
Do not merge this. I just realized what a horrible mistake it is to let you specify the name of the element to wrap in in a parameter. I'll fix this in a minute. |
64dece7 to
e6f8b62
Compare
|
That should fix the problem with the tags; it now only allows a given set of tags (code and kbd at the moment). |
There was a problem hiding this comment.
Some nits that would be nice to address, otherwise r+
I verified this on https://developer.mozilla.org/en-US/docs/Web/HTML/Applying_color#Keywords
I did not test the other 70 pages which make use of this macro.
macros/SectionOnPage.ejs
Outdated
| // $2 Element name to wrap the page title (not the section name) in; | ||
| // leave blank or don't specify this parameter to not wrap the title. | ||
| // Don't include the angle brackets! Only specific tags are permitted; | ||
| // they're in the allowedWrappers array. |
There was a problem hiding this comment.
Thanks for adding docs about this parameter. I think it is also a good idea to add a sample call:
// Example call
// {{SectionOnPage("/en-US/docs/Web/CSS/color_value", "Color keywords", "code")}}
| var text = ""; | ||
| var page = wiki.getPage($0); | ||
| var title = page.title; | ||
| var title = kuma.htmlEscape(page.title); |
There was a problem hiding this comment.
If a page doesn't exist, this will output "SectionName in undefined". Not sure if we want to do something about it.
macros/SectionOnPage.ejs
Outdated
| elem = elem.toLowerCase(); | ||
| if (allowedWrappers.includes(elem)) { | ||
| title = `<${elem}>${title}</${elem}>` |
There was a problem hiding this comment.
nit: semicolon at the end. (thanks for using template literals!).
The SectionOnPage macro produces corrupt HTML if the title of the page includes angle brackets. This patch fixes that; it also adds support for a third parameter that lets you specify the name of an element to wrap the title in (optional).
61aaf0d to
de9000a
Compare
| var summary = (page && page.summary) ? mdn.escapeQuotes(page.summary) : | ||
| localize(commonLocalStrings, "summary"); | ||
| if (!title || title == undefined || title == "undefined") { |
There was a problem hiding this comment.
I think you want if (title === undefined) here.
title == "undefined" looks like you wanted to actually write typeof title === "undefined", which I think isn't needed as the title variable has been declared, so there is no risk of a ReferenceError here.
There was a problem hiding this comment.
Actually, I do in fact mean title == "undefined" here. Turns out kuma.htmlEscape(page.title) returns the string "undefined" if the page doesn't exist.
Add updated translations for: * es-MX - Spanish (Mexico) * ka - Georgian * te - Telegu Add kumascript PRs: * mdn/kumascript#320 - L10n:Common, SectionOnPage: Fix escaping * mdn/kumascript#346 - Push tagged images to quay.io * mdn/kumascript#352 - SpecName, spec2: Add Visual Viewport API * mdn/kumascript#353 - LegacyAddonsNotice: Add French * mdn/kumascript#354 - JsSidebar: Add Client-side web APIs
The SectionOnPage macro produces corrupt HTML if the title of the page includes angle
brackets. This patch fixes that; it also adds support for a third parameter that lets
you specify the name of an element to wrap the title in (optional).