Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/398135 )
Change subject: Revert "Update section validation logic"
......................................................................
Revert "Update section validation logic"
This reverts commit 6181d6a191dce0ca581475abd373d23348141748.
Change-Id: Ib65e2d670263f37b34257928f9b94f8884e6e64b
---
M lib/parsoid-access.js
M lib/parsoidSections.js
M lib/parsoidSectionsUsingSectionTags.js
M test/lib/parsoid/parsoid-sections-section-elements-tests.js
4 files changed, 31 insertions(+), 57 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/35/398135/1
diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index 3ae3308..338ce33 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -119,7 +119,7 @@
transforms.stripUnneededMarkup(doc, legacy);
parsoidSections.addSectionDivs(doc);
- page.sections = parsoidSections.getSectionsText(doc, req.logger);
+ page.sections = parsoidSections.getSectionsText(doc);
return page;
});
}
diff --git a/lib/parsoidSections.js b/lib/parsoidSections.js
index f8e21d6..dcdbb72 100644
--- a/lib/parsoidSections.js
+++ b/lib/parsoidSections.js
@@ -27,14 +27,13 @@
* Gets the sections array, including HTML string and metadata for all sections
* (id, anchor, line, toclevel).
* @param {!document} doc the parsed DOM Document of the Parsoid output
- * @param {!Logger} logger the app's bunyan logger
* @return {!sections[]} an array of section JSON elements
*/
-function getSectionsText(doc, logger) {
+function getSectionsText(doc) {
if (!hasParsoidSections(doc)) {
return parsoidSectionsUsingDivs.getSectionsText(doc);
} else {
- return parsoidSectionsUsingSectionTags.getSectionsText(doc, logger);
+ return parsoidSectionsUsingSectionTags.getSectionsText(doc);
}
}
diff --git a/lib/parsoidSectionsUsingSectionTags.js
b/lib/parsoidSectionsUsingSectionTags.js
index ce27178..76e471d 100644
--- a/lib/parsoidSectionsUsingSectionTags.js
+++ b/lib/parsoidSectionsUsingSectionTags.js
@@ -1,6 +1,8 @@
'use strict';
const parsoidDomUtils = require('parsoid-dom-utils');
+// const sUtil = require('./util');
+// const HTTPError = sUtil.HTTPError;
const NodeType = require('./nodeType');
/**
@@ -37,13 +39,7 @@
}
}
-// skip lead sections since they don't have headings;
-// no line and no anchor means incomplete section
-function shouldLogInvalidSectionWarning(sectionObj) {
- return sectionObj.id >= 1 && !sectionObj.line && !sectionObj.anchor;
-}
-
-function validatePreviousSection(logger, allSections, sectionObj) {
+/* function throwIfPreviousSectionIsIncomplete(allSections, sectionObj) {
if (allSections.length === 0) {
return;
}
@@ -51,29 +47,30 @@
if (!sectionObj) {
sectionObj = allSections[allSections.length - 1];
}
-
- if (shouldLogInvalidSectionWarning(sectionObj)) {
- logger.warn({
- warning: 'invalid_section',
- title: 'Found section without expected heading',
+ // skip lead sections since they don't have headings;
+ // no line and no anchor means incomplete section
+ if (allSections.length > 1 && !sectionObj.line && !sectionObj.anchor) {
+ throw new HTTPError({
+ status: 502,
+ type: 'unsupported_section',
+ title: 'Section structure not supported',
detail: `Cannot find heading for section number ${sectionObj.id}.`
});
}
-}
+} */
/**
* Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
* In this case, starts a new section object when a <section> tag is
encountered that is a
* direct descendant
- * @param {!Logger} logger the app's bunyan logger
* @param {!DOMNode} node the node to visit
* @param {!Object[]} allSections the array containing the results
* @param {!Object} state some state to pass around
*/
-function visit(logger, node, allSections, state) {
+function visit(node, allSections, state) {
let sectionObj = allSections.length > 0 ? allSections[allSections.length -
1] : undefined;
if (node.tagName === 'SECTION') {
- validatePreviousSection(logger, allSections, sectionObj);
+ // throwIfPreviousSectionIsIncomplete(allSections, sectionObj);
sectionObj = { id: getSectionNumber(node), text: '' };
allSections.push(sectionObj);
@@ -102,17 +99,16 @@
* This started out with a traditional iterative DFS algorithm but added a
boolean to stop
* descending so we don't duplicate content since elsewhere outerHTML is used
(which contains
* content of sub-nodes).
- * @param {!Logger} logger the app's bunyan logger
* @param {!DOMElement} rootElement the root of the DOM tree which needs to be
traversed
* @param {!Object[]} allSections holds the results
*/
-function traverseDF(logger, rootElement, allSections) {
+function traverseDF(rootElement, allSections) {
const state = {};
let nodesToVisit = [ rootElement ];
while (nodesToVisit.length > 0) {
const currentNode = nodesToVisit.shift();
- visit(logger, currentNode, allSections, state);
+ visit(currentNode, allSections, state);
if (!state.pauseDescent) {
nodesToVisit = [
...(currentNode.childNodes || []), // depth first
@@ -121,12 +117,12 @@
}
}
ensureLeadSection(allSections);
- validatePreviousSection(logger, allSections);
+ // throwIfPreviousSectionIsIncomplete(allSections);
}
-function getSectionsText(doc, logger) {
+function getSectionsText(doc) {
const allSections = [];
- traverseDF(logger, doc.body, allSections);
+ traverseDF(doc.body, allSections);
return allSections;
}
@@ -134,7 +130,6 @@
addSectionTags,
getSectionsText,
testing: {
- parseSections: traverseDF,
- shouldLogInvalidSectionWarning
+ parseSections: traverseDF
}
};
diff --git a/test/lib/parsoid/parsoid-sections-section-elements-tests.js
b/test/lib/parsoid/parsoid-sections-section-elements-tests.js
index ecfb94c..77cb023 100644
--- a/test/lib/parsoid/parsoid-sections-section-elements-tests.js
+++ b/test/lib/parsoid/parsoid-sections-section-elements-tests.js
@@ -1,13 +1,11 @@
-/* eslint-disable prefer-template */
-
'use strict';
const assert = require('../../utils/assert.js');
const domino = require('domino');
const parsoidSectionsUsingSectionTags =
require('../../../lib/parsoidSectionsUsingSectionTags');
-const shouldWarn =
parsoidSectionsUsingSectionTags.testing.shouldLogInvalidSectionWarning;
describe('lib:parsoid-sections (section elements)', function() {
+ /* eslint-disable prefer-template */
this.timeout(20000); // eslint-disable-line no-invalid-this
@@ -121,32 +119,14 @@
assertSection0(sections, sectionInDiv);
});
- it('should not warn for page containing only a lead section', () => {
- const allSections = [ { id: 0 } ];
- assert.ok(!shouldWarn(allSections[0]));
+ it.skip('non-lead section without heading tag should throw error', () => {
+ const doc = domino.createDocument(
+ '<section data-mw-section-id="0">text0</section>' +
+ '<section data-mw-section-id="1">text1</section>');
+ assert.throws(() => {
+ parsoidSectionsUsingSectionTags.getSectionsText(doc);
+ }, /unsupported_section/);
});
- it('should warn for non-lead section without heading tag', () => {
- const allSections = [ { id: 0 }, { id: 1 } ];
- assert.ok(shouldWarn(allSections[1]));
- });
-
- it('should not warn if id & anchor are found for all sections after the
lead section', () => {
- const allSections = [ { id: 0 }, { id: 1, line: 'Foo', anchor: 'Foo' }
];
- assert.ok(!shouldWarn(allSections[1]));
- });
-
- it('should not warn for non-lead non-editable section without heading
tag', () => {
- const allSections = [ { id: 0 }, { id: -1 } ];
- assert.ok(!shouldWarn(allSections[1]));
- });
-
- it('should not warn if a non-editable section precedes the true lead
section', () => {
- const allSections = [ { id: -1 }, { id: 0 }, { id: 1, line: 'Foo',
anchor: 'Foo' } ];
- assert.ok(!shouldWarn(allSections[1]));
- });
-
- it('should throw if sectionObj is invalid', () => {
- assert.throws(() => { shouldWarn(undefined); }, /TypeError/);
- });
+ /* eslint-enable prefer-template */
});
--
To view, visit https://gerrit.wikimedia.org/r/398135
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib65e2d670263f37b34257928f9b94f8884e6e64b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <mhollo...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
