jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/309170 )
Change subject: Add class to IPA container, do not use inline style ...................................................................... Add class to IPA container, do not use inline style This is error prone and there are certain articles where this will display incorrectly. Using a class allows consumers of the API to handle this edge case as they wish. This is applied inside the new /formatted endpoint but not inside the old mobile content service for backwards compatibility Additional changes: * Any instance of removeNodes has been renamed and repurposed as a legacy parameter. Bug: T141835 Change-Id: Ica509ac9d2b643be044f4b0be3181a9bc7bc911d --- M lib/parsoid-access.js M lib/transformations/hideIPA.js M lib/transforms.js M routes/mobile-sections.js M test/features/mobile-sections/pagecontent-v2.js M test/features/mobile-sections/pagecontent.js 6 files changed, 72 insertions(+), 30 deletions(-) Approvals: BearND: Looks good to me, approved jenkins-bot: Verified diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js index 045581f..1cc290e 100644 --- a/lib/parsoid-access.js +++ b/lib/parsoid-access.js @@ -129,9 +129,12 @@ /** * @param {Object} app: the application object * @param {Object} req: the request object + * @param {Boolean} [legacy] if enabled will apply additional transformations + * including a legacy version of relocation of first paragraph + * and hiding IPA via an inline style rather than clas. * @return {promise} Returns a promise to retrieve the page content from Parsoid */ -function pageContentPromise(app, req) { +function pageContentPromise(app, req, legacy) { return getParsoidHtml(app, req) .then((response) => { const page = { revision: getRevisionFromEtag(response.headers) }; @@ -142,7 +145,7 @@ parseProperty.parseGeo(doc, page); parseProperty.parseSpokenWikipedia(doc, page); - transforms.stripUnneededMarkup(doc); + transforms.stripUnneededMarkup(doc, legacy); addSectionDivs(doc); transforms.addRequiredMarkup(doc); diff --git a/lib/transformations/hideIPA.js b/lib/transformations/hideIPA.js index 0b7e4b6..bc94e43 100644 --- a/lib/transformations/hideIPA.js +++ b/lib/transformations/hideIPA.js @@ -10,7 +10,11 @@ 'use strict'; -function hideIPA(content) { +/** + * @param {Document} content + * @param {Boolean} [legacy] + */ +function hideIPA(content, legacy) { const spans = content.querySelectorAll("span.IPA"); for (let i = 0; i < spans.length; i++) { const parentSpan = spans[i].parentNode; @@ -41,8 +45,12 @@ containerSpan.appendChild(buttonDiv); containerSpan.appendChild(parentSpan); - // set initial visibility - parentSpan.style.display = 'none'; + if (legacy) { + parentSpan.style.display = 'none'; + } + // markup parent with class so that it can be hidden if required + const parentSpanClass = parentSpan.className; + parentSpan.className = parentSpanClass ? `${parentSpanClass} mcs-ipa` : 'mcs-ipa'; } } diff --git a/lib/transforms.js b/lib/transforms.js index b660912..481bd90 100644 --- a/lib/transforms.js +++ b/lib/transforms.js @@ -194,9 +194,9 @@ /** * Destructive, non-Parsoid-specific transforms previously performed in the app. */ -function _removeUnwantedWikiContentForApp(doc) { +function _removeUnwantedWikiContentForApp(doc, legacy) { hideRedLinks.hideRedLinks(doc); - hideIPA.hideIPA(doc); + hideIPA.hideIPA(doc, legacy); } // /** @@ -222,12 +222,12 @@ /** * Nukes stuff from the DOM we don't want for pages from Parsoid. */ -transforms.stripUnneededMarkup = function(doc) { +transforms.stripUnneededMarkup = function(doc, legacy) { _runAllSectionsTransforms(doc); // runLeadSectionTransforms(doc); _applyOptionalParsoidSpecificTransformations(doc); - _removeUnwantedWikiContentForApp(doc); + _removeUnwantedWikiContentForApp(doc, legacy); }; transforms.addRequiredMarkup = function(doc) { diff --git a/routes/mobile-sections.js b/routes/mobile-sections.js index 75bd57c..1340913 100644 --- a/routes/mobile-sections.js +++ b/routes/mobile-sections.js @@ -77,13 +77,13 @@ /* * @param {Object} input - * @param {Boolean} [removeNodes] whether to remove nodes from the lead text + * @param {Boolean} [legacy] whether to perform legacy transformations * @return {Object} lead json */ -function buildLead(input, removeNodes) { +function buildLead(input, legacy) { const lead = domino.createDocument(input.page.sections[0].text); - if (!removeNodes) { + if (legacy) { // Move the first good paragraph up for any page except main pages. // It's ok to do unconditionally since we throw away the page // content if this turns out to be a main page. @@ -91,16 +91,16 @@ // TODO: should we also exclude file and other special pages? transforms.relocateFirstParagraph(lead); } - const hatnotes = transforms.extractHatnotes(lead, removeNodes); + const hatnotes = transforms.extractHatnotes(lead, !legacy); const pronunciation = parse.parsePronunciation(lead, input.meta.displaytitle); - const issues = transforms.extractPageIssues(lead, removeNodes); + const issues = transforms.extractPageIssues(lead, !legacy); let infobox; let intro; let sections; let text; - if (removeNodes) { + if (!legacy) { if (input.page.sections.length > 1) { infobox = transforms.extractInfobox(lead); intro = transforms.extractLeadIntroduction(lead); @@ -175,12 +175,12 @@ /* * @param {Object} input - * @param {Boolean} [removeNodes] whether to remove nodes from the lead text + * @param {Boolean} [legacy] whether to perform legacy transformations * @return {Object} */ -function buildAll(input, removeNodes) { +function buildAll(input, legacy) { return { - lead: buildLead(input, removeNodes), + lead: buildLead(input, legacy), remaining: buildRemaining(input) }; } @@ -199,9 +199,17 @@ }); } -function buildAllResponse(req, res, removeNodes) { +/** + * @param {Request} req + * @param {Response} res + * @param {Boolean} [legacy] when true MCS will + * not apply legacy transformations that we are in the process + * of deprecating. + * @return {BBPromise} + */ +function buildAllResponse(req, res, legacy) { return BBPromise.props({ - page: parsoid.pageContentPromise(app, req), + page: parsoid.pageContentPromise(app, req, legacy), meta: pageMetadataPromise(req) }).then((response) => { if (response.meta.mainpage) { @@ -209,7 +217,7 @@ } return response; }).then((response) => { - response = buildAll(response, removeNodes); + response = buildAll(response, legacy); res.status(200); mUtil.setETag(res, response.lead.revision); mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections); @@ -217,9 +225,9 @@ }); } -function buildLeadResponse(req, res, removeNodes) { +function buildLeadResponse(req, res, legacy) { return BBPromise.props({ - page: parsoid.pageContentPromise(app, req), + page: parsoid.pageContentPromise(app, req, legacy), meta: pageMetadataPromise(req) }).then((response) => { if (response.meta.mainpage) { @@ -227,7 +235,7 @@ } return response; }).then((response) => { - response = buildLead(response, removeNodes); + response = buildLead(response, legacy); res.status(200); mUtil.setETag(res, response.revision); mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections); @@ -239,7 +247,7 @@ * Gets the mobile app version of a given wiki page. */ router.get('/mobile-sections/:title/:revision?', (req, res) => { - return buildAllResponse(req, res, false); + return buildAllResponse(req, res, true); }); /** @@ -247,7 +255,7 @@ * Gets the lead section for the mobile app version of a given wiki page. */ router.get('/mobile-sections-lead/:title/:revision?', (req, res) => { - return buildLeadResponse(req, res, false); + return buildLeadResponse(req, res, true); }); /** @@ -256,7 +264,7 @@ */ router.get('/mobile-sections-remaining/:title/:revision?', (req, res) => { return BBPromise.props({ - page: parsoid.pageContentPromise(app, req) + page: parsoid.pageContentPromise(app, req, true) }).then((response) => { res.status(200); mUtil.setETag(res, response.page.revision); @@ -271,7 +279,7 @@ */ router.get('/references/:title/:revision?', (req, res) => { return BBPromise.props({ - page: parsoid.pageContentPromise(app, req) + page: parsoid.pageContentPromise(app, req, false) }).then((response) => { res.status(200); mUtil.setETag(res, response.page.revision); @@ -285,7 +293,7 @@ * Gets a formatted version of a given wiki page rather than a blob of wikitext. */ router.get('/formatted/:title/:revision?', (req, res) => { - return buildAllResponse(req, res, true); + return buildAllResponse(req, res, false); }); /** @@ -293,7 +301,7 @@ * Gets a formatted version of a given wiki page rather than a blob of wikitext. */ router.get('/formatted-lead/:title/:revision?', (req, res) => { - return buildLeadResponse(req, res, true); + return buildLeadResponse(req, res, false); }); module.exports = function(appObj) { diff --git a/test/features/mobile-sections/pagecontent-v2.js b/test/features/mobile-sections/pagecontent-v2.js index 8276acd..36fa24f 100644 --- a/test/features/mobile-sections/pagecontent-v2.js +++ b/test/features/mobile-sections/pagecontent-v2.js @@ -100,4 +100,14 @@ 'List element is bundled into paragraph'); }); }); + + it('Page with IPA content', () => { + const uri = `${server.config.uri}en.wikipedia.org/v1/page/formatted/Sunderland_A.F.C.`; + return preq.get({ uri }) + .then((res) => { + const text = res.body.lead.intro; + const expected = '<span class="nowrap mcs-ipa"><span class="noexcerpt">'; + assert.ok(text.indexOf(expected) > -1, 'mcs-ipa class is found'); + }); + }); }); diff --git a/test/features/mobile-sections/pagecontent.js b/test/features/mobile-sections/pagecontent.js index 82e656a..eb67882 100644 --- a/test/features/mobile-sections/pagecontent.js +++ b/test/features/mobile-sections/pagecontent.js @@ -246,4 +246,17 @@ assert.equal(res.status, 200); }); }); + + it('Page with IPA content', () => { + const title = 'Sunderland_A.F.C.'; + const uri = `${server.config.uri}en.wikipedia.org/v1/page/mobile-sections/${title}`; + return preq.get({ uri }) + .then((res) => { + const text = res.body.lead.sections[0].text; + const expected = 'style="display: none;"><span class="noexcerpt">'; + assert.equal(res.status, 200); + assert.ok(text.indexOf(expected) > -1, + 'IPA information is wrapped in hidden container'); + }); + }); }); -- To view, visit https://gerrit.wikimedia.org/r/309170 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ica509ac9d2b643be044f4b0be3181a9bc7bc911d Gerrit-PatchSet: 11 Gerrit-Project: mediawiki/services/mobileapps Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org> Gerrit-Reviewer: Dbrant <dbr...@wikimedia.org> Gerrit-Reviewer: Fjalapeno <cfl...@wikimedia.org> Gerrit-Reviewer: GWicke <gwi...@wikimedia.org> Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Jhernandez <jhernan...@wikimedia.org> Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org> Gerrit-Reviewer: Mhurd <mh...@wikimedia.org> Gerrit-Reviewer: Mobrovac <mobro...@wikimedia.org> Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits