Mholloway has uploaded a new change for review.
https://gerrit.wikimedia.org/r/278403
Change subject: Hide links to missing pages, Take 2
......................................................................
Hide links to missing pages, Take 2
Gets a list of links with missing pages from the VisualEditor extension API
and hides them, so we don't display dead links in the app.
This is necessary because unlike the PHP parser, Parsoid does not flag
links to nonexistent pages, so we can't just search them by class and
remove them as we did with PHP parser HTML.
Note that this will only work for wikis with VisualEditor enabled.
An earlier version of this patch was deployed and then reverted because of
memory problems in production; I believe this was because the
hideDeadLinks function was poorly designed.
Bug: T119266
Change-Id: Ib1e30775c4743e2f6dc5ea3151a0dacb51cbb001
---
M lib/mwapi.js
M lib/parsoid-access.js
A lib/transformations/hideDeadLinks.js
D lib/transformations/hideRedLinks.js
M lib/transforms.js
M test/features/mobile-sections/pagecontent.js
6 files changed, 99 insertions(+), 47 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/03/278403/1
diff --git a/lib/mwapi.js b/lib/mwapi.js
index 49a50ff..689a95f 100644
--- a/lib/mwapi.js
+++ b/lib/mwapi.js
@@ -7,6 +7,7 @@
var preq = require('preq');
var sUtil = require('../lib/util');
var Template = require('swagger-router').Template;
+var BBPromise = require('bluebird');
var HTTPError = sUtil.HTTPError;
@@ -88,6 +89,29 @@
checkResponseStatus(response);
return response;
});
+}
+
+/**
+ * Get list of dead links for the page.
+ *
+ * @param {Object} app the application object
+ * @param {Object} req the request object
+ * @return {Array} list of dead link titles (always present, may be empty)
+ */
+function getDeadLinks(app, req) {
+ return apiGet(app, req, {
+ "action": "visualeditor",
+ "paction": "metadata",
+ "format": "json",
+ "page": req.params.title
+ }).then(function (response) {
+ // Return an empty array if there's an error or VE is not enabled for
+ // this wiki 109
+ return BBPromise.resolve(response.body.visualeditor
+ && response.body.visualeditor.links
+ && response.body.visualeditor.links.missing
+ || []);
+ });
}
/**
@@ -195,6 +219,7 @@
module.exports = {
apiGet: apiGet,
getMetadata: getMetadata,
+ getDeadLinks: getDeadLinks,
getAllSections: getAllSections,
buildLeadImageUrls: buildLeadImageUrls,
checkResponseStatus: checkResponseStatus,
diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index a2e280e..16929a6 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -14,8 +14,10 @@
var parseProperty = require('./parseProperty');
var parseDefinition = require('./parseDefinition');
var relocateFirstParagraph =
require('./transformations/relocateFirstParagraph');
+var hideDeadLinks = require('../lib/transformations/hideDeadLinks');
var transforms = require('./transforms');
var HTTPError = sUtil.HTTPError;
+var BBPromise = require('bluebird');
var REDIRECT_REGEXP = /<link rel="mw:PageProp\/redirect" href="\.\/([^"]+)"/;
@@ -217,30 +219,36 @@
* @return {promise} Returns a promise to retrieve the page content from
Parsoid
*/
function pageContentPromise(app, req) {
- return getContent(app, req)
- .then(function (response) {
- var page = { revision: getRevisionFromEtag(response.headers) };
- var doc = domino.createDocument(response.body);
+ return BBPromise.props({
+ content: getContent(app, req),
+ deadlinks: mwapi.getDeadLinks(app, req)
+ }).then(function (response) {
+ var page = { revision: getRevisionFromEtag(response.content.headers) };
+ var doc = domino.createDocument(response.content.body);
- // Note: these properties must be obtained before stripping markup
- page.lastmodified = getModified(doc);
- parseProperty.parseGeo(doc, page);
- parseProperty.parseSpokenWikipedia(doc, page);
+ // Note: these properties must be obtained before stripping markup
+ page.lastmodified = getModified(doc);
+ parseProperty.parseGeo(doc, page);
+ parseProperty.parseSpokenWikipedia(doc, page);
- transforms.stripUnneededMarkup(doc);
- addSectionDivs(doc);
- transforms.addRequiredMarkup(doc);
+ if (response.deadlinks.length !== 0) {
+ hideDeadLinks(doc, response.deadlinks);
+ }
- // 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.
- //
- // TODO: should we also exclude file and other special pages?
- relocateFirstParagraph(doc);
+ transforms.stripUnneededMarkup(doc);
+ addSectionDivs(doc);
+ transforms.addRequiredMarkup(doc);
- page.sections = getSectionsText(doc);
- return page;
- });
+ // 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.
+ //
+ // TODO: should we also exclude file and other special pages?
+ relocateFirstParagraph(doc);
+
+ page.sections = getSectionsText(doc);
+ return page;
+ });
}
/*
diff --git a/lib/transformations/hideDeadLinks.js
b/lib/transformations/hideDeadLinks.js
new file mode 100644
index 0000000..72f5f5a
--- /dev/null
+++ b/lib/transformations/hideDeadLinks.js
@@ -0,0 +1,36 @@
+/**
+ * A function to remove dead links from Parsoid-generated HTML, which unlike
PHP
+ * parser HTML does not flag links to missing pages. For this reason, we
+ * deviate from the old handling of red links in hideRedLinks.js.
+ */
+
+'use strict';
+
+var escape = require('css.escape');
+
+/**
+ * @param {Object} doc a Parsoid DOM
+ * @param {Object} deadLinks an array of titles of nonexistent wiki pages
+ */
+function hideDeadLinks(doc, deadLinks) {
+ if (deadLinks.length === 0) {
+ return;
+ }
+
+ for (var i = 0; i < deadLinks.length; i++) {
+ // Exclude titles containing apostrophes since they break
selection
+ if (deadLinks[i].indexOf("'") !== -1) {
+ continue;
+ }
+
+ var deadLink = doc.querySelector('a[title=' + deadLinks[i] +
']');
+ if (deadLink) {
+ var replacementSpan = doc.createElement('span');
+ replacementSpan.innerHTML = deadLink.innerHTML;
+ replacementSpan.setAttribute('class',
deadLink.getAttribute('class'));
+ deadLink.parentNode.replaceChild(replacementSpan,
deadLink);
+ }
+ }
+}
+
+module.exports = hideDeadLinks;
diff --git a/lib/transformations/hideRedLinks.js
b/lib/transformations/hideRedLinks.js
deleted file mode 100644
index e914384..0000000
--- a/lib/transformations/hideRedLinks.js
+++ /dev/null
@@ -1,24 +0,0 @@
-/**
- * DOM transformation shared with app. Let's keep this in sync with the app.
- * Last sync: Android repo 3d5b441 www/js/transforms/hideRedLinks.js
- *
- * The main change from the original Android app file is to use
- * content.createElement() instead of document.createElement().
- */
-
-'use strict';
-
-function hideRedLinks(content) {
- var redLinks = content.querySelectorAll( 'a.new' );
- for ( var i = 0; i < redLinks.length; i++ ) {
- var redLink = redLinks[i];
- var replacementSpan = content.createElement( 'span' );
- replacementSpan.innerHTML = redLink.innerHTML;
- replacementSpan.setAttribute( 'class', redLink.getAttribute(
'class' ) );
- redLink.parentNode.replaceChild( replacementSpan, redLink );
- }
-}
-
-module.exports = {
- hideRedLinks: hideRedLinks
-};
diff --git a/lib/transforms.js b/lib/transforms.js
index 3e2c773..8de70d5 100644
--- a/lib/transforms.js
+++ b/lib/transforms.js
@@ -8,7 +8,7 @@
var domino = require('domino');
var util = require('util');
var anchorPopUpMediaTransforms =
require('./transformations/anchorPopUpMediaTransforms');
-var hideRedLinks = require('./transformations/hideRedLinks');
+var hideDeadLinks = require('./transformations/hideDeadLinks');
var hideIPA = require('./transformations/hideIPA');
var setMathFormulaImageMaxWidth =
require('./transformations/setMathFormulaImageMaxWidth');
@@ -200,7 +200,6 @@
* Destructive, non-Parsoid-specific transforms previously performed in the
app.
*/
function removeUnwantedWikiContentForApp(doc) {
- hideRedLinks.hideRedLinks(doc);
hideIPA.hideIPA(doc);
}
@@ -239,7 +238,7 @@
function runMainPageDomTransforms(text) {
var doc = domino.createDocument(text);
- runAllSectionsTransforms(doc);
+ runAllSectionsTransforms(doc, []);
//runLeadSectionTransforms(doc);
return doc.body.innerHTML;
diff --git a/test/features/mobile-sections/pagecontent.js
b/test/features/mobile-sections/pagecontent.js
index d881bed..3ad4f5a 100644
--- a/test/features/mobile-sections/pagecontent.js
+++ b/test/features/mobile-sections/pagecontent.js
@@ -119,4 +119,12 @@
assert.deepEqual(res.status, 200);
});
});
+ it('Redlink should be removed', function() {
+ return preq.get({ uri: server.config.uri +
'en.wikipedia.org/v1/page/mobile-sections/User:Mhollo%2Fredlink_test' })
+ .then(function(res) {
+ var leadSection = res.body.lead.sections[0].text;
+ assert.ok(leadSection.indexOf('redlink') > -1, '"redlink" text
exists');
+ assert.deepEqual(leadSection.indexOf('href'), -1, 'no link in
present in output');
+ })
+ })
});
--
To view, visit https://gerrit.wikimedia.org/r/278403
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib1e30775c4743e2f6dc5ea3151a0dacb51cbb001
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
