[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Do not relocate first paragraph in new endpoint

2016-10-13 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Do not relocate first paragraph in new endpoint
..


Do not relocate first paragraph in new endpoint

This moves the logic for relocating a lead paragraph into the routes
that serve it. Previously we did this for all endpoints but it doesn't
seem necessary for the media endpoint.

This means the transformation is not invoked in the new endpoint.

The method is also updated to take the lead as an input to avoid an
addition node lookup.

Change-Id: Iedd38dca724d62e7b9a86cf04d9858b213aae27c
---
M lib/parsoid-access.js
M lib/transformations/relocateFirstParagraph.js
M lib/transforms.js
M routes/mobile-sections.js
M routes/mobile-summary.js
5 files changed, 21 insertions(+), 15 deletions(-)

Approvals:
  BearND: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index abbd482..420589b 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -13,7 +13,6 @@
 var parseSection = require('./parseSection');
 var parseProperty = require('./parseProperty');
 var parseDefinition = require('./parseDefinition');
-var relocateFirstParagraph = 
require('./transformations/relocateFirstParagraph');
 var transforms = require('./transforms');
 var HTTPError = sUtil.HTTPError;
 
@@ -151,13 +150,6 @@
 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);
 
 page.sections = getSectionsText(doc);
 return page;
diff --git a/lib/transformations/relocateFirstParagraph.js 
b/lib/transformations/relocateFirstParagraph.js
index 5d5dcf1..b910600 100644
--- a/lib/transformations/relocateFirstParagraph.js
+++ b/lib/transformations/relocateFirstParagraph.js
@@ -77,12 +77,15 @@
 return leadSpan;
 }
 
-// Move the first non-empty paragraph (and related elements) of the lead 
section
-// to the top of the lead section.
-// This will have the effect of shifting the infobox and/or any images at the 
top of the page
-// below the first paragraph, allowing the user to start reading the page 
right away.
-function moveFirstGoodParagraphUp(doc) {
-var leadSection = doc.getElementById('section_0');
+/**
+ * Move the first non-empty paragraph (and related elements) of the lead 
section
+ * to the top of the lead section.
+ * This will have the effect of shifting the infobox and/or any images at the 
top of the page
+ * below the first paragraph, allowing the user to start reading the page 
right away.
+ * @param {Document} lead a document that has the lead section as the body
+ */
+function moveFirstGoodParagraphUp(lead) {
+var leadSection = lead.body;
 if ( !leadSection ) {
 return;
 }
@@ -92,7 +95,7 @@
 return;
 }
 
-var leadSpan = createLeadSpan( doc, childNodes );
+var leadSpan = createLeadSpan( lead, childNodes );
 leadSection.insertBefore( leadSpan, leadSection.firstChild );
 }
 
diff --git a/lib/transforms.js b/lib/transforms.js
index aca4950..ee3545b 100644
--- a/lib/transforms.js
+++ b/lib/transforms.js
@@ -12,6 +12,7 @@
 var hideIPA = require('./transformations/hideIPA');
 var extractHatnotes = require('./transformations/extractHatnotes');
 var markReferenceSections = require('./transformations/markReferenceSections');
+var relocateFirstParagraph = 
require('./transformations/relocateFirstParagraph');
 
 var transforms = {};
 
@@ -259,6 +260,7 @@
 transforms._rmBracketSpans = _rmBracketSpans;
 transforms._rmElementsWithSelectors = _rmElementsWithSelectors;
 
+transforms.relocateFirstParagraph = relocateFirstParagraph;
 transforms.extractHatnotes = extractHatnotes;
 transforms.markReferenceSections = markReferenceSections;
 
diff --git a/routes/mobile-sections.js b/routes/mobile-sections.js
index b86a0ef..6667469 100644
--- a/routes/mobile-sections.js
+++ b/routes/mobile-sections.js
@@ -84,6 +84,14 @@
  */
 function buildLead(input, removeNodes) {
 var lead = domino.createDocument(input.page.sections[0].text);
+if ( !removeNodes ) {
+// 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?
+transforms.relocateFirstParagraph(lead);
+}
 var hatnotes = transforms.extractHatnotes(lead, removeNodes);
 
 // update text after extractions have taken place
diff --git a/routes/mobile-summary.js b

[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Do not relocate first paragraph in new endpoint

2016-10-10 Thread Jdlrobson (Code Review)
Jdlrobson has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/315126

Change subject: Do not relocate first paragraph in new endpoint
..

Do not relocate first paragraph in new endpoint

This moves the logic for relocating a lead paragraph into the routes
that serve it. Previously we did this for all endpoints but it doesn't
seem necessary for the media endpoint.

This means the transformation is not invoked in the new endpoint.

The method is also updated to take the lead as an input to avoid an
addition node lookup.

Change-Id: Iedd38dca724d62e7b9a86cf04d9858b213aae27c
---
M lib/parsoid-access.js
M lib/transformations/relocateFirstParagraph.js
M lib/transforms.js
M routes/mobile-sections.js
M routes/mobile-summary.js
5 files changed, 21 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps 
refs/changes/26/315126/1

diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index abbd482..420589b 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -13,7 +13,6 @@
 var parseSection = require('./parseSection');
 var parseProperty = require('./parseProperty');
 var parseDefinition = require('./parseDefinition');
-var relocateFirstParagraph = 
require('./transformations/relocateFirstParagraph');
 var transforms = require('./transforms');
 var HTTPError = sUtil.HTTPError;
 
@@ -151,13 +150,6 @@
 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);
 
 page.sections = getSectionsText(doc);
 return page;
diff --git a/lib/transformations/relocateFirstParagraph.js 
b/lib/transformations/relocateFirstParagraph.js
index 5d5dcf1..b910600 100644
--- a/lib/transformations/relocateFirstParagraph.js
+++ b/lib/transformations/relocateFirstParagraph.js
@@ -77,12 +77,15 @@
 return leadSpan;
 }
 
-// Move the first non-empty paragraph (and related elements) of the lead 
section
-// to the top of the lead section.
-// This will have the effect of shifting the infobox and/or any images at the 
top of the page
-// below the first paragraph, allowing the user to start reading the page 
right away.
-function moveFirstGoodParagraphUp(doc) {
-var leadSection = doc.getElementById('section_0');
+/**
+ * Move the first non-empty paragraph (and related elements) of the lead 
section
+ * to the top of the lead section.
+ * This will have the effect of shifting the infobox and/or any images at the 
top of the page
+ * below the first paragraph, allowing the user to start reading the page 
right away.
+ * @param {Document} lead a document that has the lead section as the body
+ */
+function moveFirstGoodParagraphUp(lead) {
+var leadSection = lead.body;
 if ( !leadSection ) {
 return;
 }
@@ -92,7 +95,7 @@
 return;
 }
 
-var leadSpan = createLeadSpan( doc, childNodes );
+var leadSpan = createLeadSpan( lead, childNodes );
 leadSection.insertBefore( leadSpan, leadSection.firstChild );
 }
 
diff --git a/lib/transforms.js b/lib/transforms.js
index c20bfa9..08ce2e1 100644
--- a/lib/transforms.js
+++ b/lib/transforms.js
@@ -11,6 +11,7 @@
 var hideRedLinks = require('./transformations/hideRedLinks');
 var hideIPA = require('./transformations/hideIPA');
 var extractHatnotes = require('./transformations/extractHatnotes');
+var relocateFirstParagraph = 
require('./transformations/relocateFirstParagraph');
 
 var transforms = {};
 
@@ -258,6 +259,7 @@
 transforms._rmBracketSpans = _rmBracketSpans;
 transforms._rmElementsWithSelectors = _rmElementsWithSelectors;
 
+transforms.relocateFirstParagraph = relocateFirstParagraph;
 transforms.extractHatnotes = extractHatnotes;
 
 module.exports = transforms;
diff --git a/routes/mobile-sections.js b/routes/mobile-sections.js
index e1524ba..cd2d41e 100644
--- a/routes/mobile-sections.js
+++ b/routes/mobile-sections.js
@@ -84,6 +84,14 @@
  */
 function buildLead(input, removeNodes) {
 var lead = domino.createDocument(input.page.sections[0].text);
+if ( !removeNodes ) {
+// 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?
+transforms.relocateFirstParagraph(lead);
+}
 var hatnotes = transforms.extractHatnotes(lead, removeNodes);
 
 // update text after extractions have taken place
diff --git a/routes/mobile-s