Mholloway has uploaded a new change for review.
https://gerrit.wikimedia.org/r/279193
Change subject: Revert "Hide links to missing pages, Take 2"
......................................................................
Revert "Hide links to missing pages, Take 2"
This reverts commit 6ded9245c40eed9ed8189e47dc119ea6eae58514.
Still causes problems in production.
Change-Id: Ib35e70456b45de4786fb8cfc685d035662abf9e1
---
M lib/mwapi.js
M lib/parsoid-access.js
D lib/transformations/hideDeadLinks.js
A lib/transformations/hideRedLinks.js
M lib/transforms.js
M test/features/mobile-sections/pagecontent.js
6 files changed, 46 insertions(+), 95 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/93/279193/1
diff --git a/lib/mwapi.js b/lib/mwapi.js
index 72b7024..49a50ff 100644
--- a/lib/mwapi.js
+++ b/lib/mwapi.js
@@ -7,7 +7,6 @@
var preq = require('preq');
var sUtil = require('../lib/util');
var Template = require('swagger-router').Template;
-var BBPromise = require('bluebird');
var HTTPError = sUtil.HTTPError;
@@ -88,29 +87,6 @@
return preq(request).then(function(response) {
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
- return BBPromise.resolve(response.body.visualeditor
- && response.body.visualeditor.links
- && response.body.visualeditor.links.missing
- || []);
});
}
@@ -219,7 +195,6 @@
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 16929a6..a2e280e 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -14,10 +14,8 @@
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="\.\/([^"]+)"/;
@@ -219,36 +217,30 @@
* @return {promise} Returns a promise to retrieve the page content from
Parsoid
*/
function pageContentPromise(app, req) {
- 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);
+ return getContent(app, req)
+ .then(function (response) {
+ var page = { revision: getRevisionFromEtag(response.headers) };
+ var doc = domino.createDocument(response.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);
- if (response.deadlinks.length !== 0) {
- hideDeadLinks(doc, response.deadlinks);
- }
+ transforms.stripUnneededMarkup(doc);
+ addSectionDivs(doc);
+ transforms.addRequiredMarkup(doc);
- transforms.stripUnneededMarkup(doc);
- addSectionDivs(doc);
- transforms.addRequiredMarkup(doc);
+ // 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);
- // 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;
- });
+ page.sections = getSectionsText(doc);
+ return page;
+ });
}
/*
diff --git a/lib/transformations/hideDeadLinks.js
b/lib/transformations/hideDeadLinks.js
deleted file mode 100644
index f9251fe..0000000
--- a/lib/transformations/hideDeadLinks.js
+++ /dev/null
@@ -1,34 +0,0 @@
-/**
- * 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';
-
-/**
- * @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 instances = doc.querySelectorAll('a[title=' + deadLinks[i]
+ ']');
- for (var j = 0; j < instances.length; j++) {
- var replacementSpan = doc.createElement('span');
- replacementSpan.innerHTML = instances[j].innerHTML;
- replacementSpan.setAttribute('class',
instances[j].getAttribute('class'));
- instances[j].parentNode.replaceChild(replacementSpan,
instances[j]);
- }
- }
-}
-
-module.exports = hideDeadLinks;
diff --git a/lib/transformations/hideRedLinks.js
b/lib/transformations/hideRedLinks.js
new file mode 100644
index 0000000..e914384
--- /dev/null
+++ b/lib/transformations/hideRedLinks.js
@@ -0,0 +1,24 @@
+/**
+ * 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 a1a984d..3e2c773 100644
--- a/lib/transforms.js
+++ b/lib/transforms.js
@@ -8,6 +8,7 @@
var domino = require('domino');
var util = require('util');
var anchorPopUpMediaTransforms =
require('./transformations/anchorPopUpMediaTransforms');
+var hideRedLinks = require('./transformations/hideRedLinks');
var hideIPA = require('./transformations/hideIPA');
var setMathFormulaImageMaxWidth =
require('./transformations/setMathFormulaImageMaxWidth');
@@ -199,6 +200,7 @@
* Destructive, non-Parsoid-specific transforms previously performed in the
app.
*/
function removeUnwantedWikiContentForApp(doc) {
+ hideRedLinks.hideRedLinks(doc);
hideIPA.hideIPA(doc);
}
diff --git a/test/features/mobile-sections/pagecontent.js
b/test/features/mobile-sections/pagecontent.js
index 3ad4f5a..d881bed 100644
--- a/test/features/mobile-sections/pagecontent.js
+++ b/test/features/mobile-sections/pagecontent.js
@@ -119,12 +119,4 @@
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/279193
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib35e70456b45de4786fb8cfc685d035662abf9e1
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
