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
