Mholloway has uploaded a new change for review.
https://gerrit.wikimedia.org/r/245576
Change subject: Prevent lead paragraph shifting from leaving connected content
behind
......................................................................
Prevent lead paragraph shifting from leaving connected content behind
This patch updates the lead paragraph shifting logic to create a span
consisting of the first qualifying <p> element encountered and any
subsequent non-<p> elements until the next <p> is encountered.
Simply moving the first <p> element up, as we did before, may result in
elements appearing between the first paragraph as designated by
<p></p> tags and other elements (such as an unnumbered list) that may be
intended as part of the first display paragraph.
Bug: T111958
Change-Id: I41f2b16f9b5d8fcac7bef156a3f3726d7bfe36d0
---
M app/src/main/assets/bundle.js
M www/js/transforms/relocateFirstParagraph.js
2 files changed, 103 insertions(+), 52 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/76/245576/1
diff --git a/app/src/main/assets/bundle.js b/app/src/main/assets/bundle.js
index 6c994b1..194737e 100644
--- a/app/src/main/assets/bundle.js
+++ b/app/src/main/assets/bundle.js
@@ -1023,8 +1023,9 @@
} );
},{"../transformer":12,"./collapseTables":16}],20:[function(require,module,exports){
var transformer = require("../transformer");
+var utilities = require("../utilities");
-// Move the first non-empty paragraph of text to the top of the section.
+// Move the first non-empty paragraph (and related elements) to the top of the
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.
transformer.register( "moveFirstGoodParagraphUp", function() {
@@ -1043,33 +1044,58 @@
return;
}
- for ( var i = 0; i < allPs.length; i++ ) {
- var p = allPs[i];
- // Narrow down to first P which is direct child of content_block_0 DIV.
- // (Don't want to yank P from somewhere in the middle of a table!)
- if (p.parentNode !== block_0) {
- continue;
- }
- // Ensure the P being pulled up has at least a couple lines of text.
- // Otherwise silly things like a empty P or P which only contains a
- // BR tag will get pulled up (see articles on "Chemical Reaction" and
- // "Hawaii").
- // Trick for quickly determining element height:
- //
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement.offsetHeight
- // http://stackoverflow.com/a/1343350/135557
- var minHeight = 24;
- if (p.offsetHeight < minHeight){
- continue;
- }
-
- // Move the P!
- block_0.insertBefore(p.parentNode.removeChild(p), block_0.firstChild);
-
- // But only move one P!
- break;
+ var block_0_children = block_0.childNodes;
+ if (!block_0_children) {
+ return;
}
+
+ var leadSpan = createLeadSpan(block_0_children);
+ block_0.insertBefore(leadSpan, block_0.firstChild)
} );
-},{"../transformer":12}],21:[function(require,module,exports){
+
+// Create a lead span to be moved to the top of the page, consisting of the
first
+// qualifying <p> element encountered and any subsequent non-<p> elements until
+// the next <p> is encountered.
+//
+// Simply moving the first <p> element up may result in elements appearing
+// between the first paragraph as designated by <p></p> tags and other elements
+// (such as an unnumbered list) that may also be intended as part of the first
+// display paragraph. See T111958.
+function createLeadSpan( childNodes ) {
+ var leadSpan = document.createElement( 'span' );
+ var firstParagraphIndex = childNodes.length;
+
+ for (var i = 0; i < childNodes.length; i++) {
+ if (childNodes[i].tagName === 'P') {
+ // Ensure the P being pulled up has at least a couple lines of
text.
+ // Otherwise silly things like a empty P or P which only contains a
+ // BR tag will get pulled up (see articles on "Chemical Reaction"
and
+ // "Hawaii").
+ // Trick for quickly determining element height:
+ //
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement.offsetHeight
+ // http://stackoverflow.com/a/1343350/135557
+ var minHeight = 24;
+ if (childNodes[i].offsetHeight < minHeight){
+ continue;
+ }
+
+
leadSpan.appendChild(childNodes[i].parentNode.removeChild(childNodes[i]));
+ firstParagraphIndex = i;
+ break;
+ }
+ }
+
+ for (var j = firstParagraphIndex + 1; j < childNodes.length; j++) {
+ if (childNodes[j].tagName === 'P') {
+ break;
+ }
+
leadSpan.appendChild(childNodes[j].parentNode.removeChild(childNodes[j]));
+ }
+
+ return leadSpan;
+}
+
+},{"../transformer":12,"../utilities":24}],21:[function(require,module,exports){
var transformer = require("../transformer");
transformer.register( "setMathFormulaImageMaxWidth", function( content ) {
diff --git a/www/js/transforms/relocateFirstParagraph.js
b/www/js/transforms/relocateFirstParagraph.js
index 8b0f9e5..00c4d84 100644
--- a/www/js/transforms/relocateFirstParagraph.js
+++ b/www/js/transforms/relocateFirstParagraph.js
@@ -1,6 +1,7 @@
var transformer = require("../transformer");
+var utilities = require("../utilities");
-// Move the first non-empty paragraph of text to the top of the section.
+// Move the first non-empty paragraph (and related elements) to the top of the
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.
transformer.register( "moveFirstGoodParagraphUp", function() {
@@ -19,29 +20,53 @@
return;
}
- for ( var i = 0; i < allPs.length; i++ ) {
- var p = allPs[i];
- // Narrow down to first P which is direct child of content_block_0 DIV.
- // (Don't want to yank P from somewhere in the middle of a table!)
- if (p.parentNode !== block_0) {
- continue;
- }
- // Ensure the P being pulled up has at least a couple lines of text.
- // Otherwise silly things like a empty P or P which only contains a
- // BR tag will get pulled up (see articles on "Chemical Reaction" and
- // "Hawaii").
- // Trick for quickly determining element height:
- //
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement.offsetHeight
- // http://stackoverflow.com/a/1343350/135557
- var minHeight = 24;
- if (p.offsetHeight < minHeight){
- continue;
- }
-
- // Move the P!
- block_0.insertBefore(p.parentNode.removeChild(p), block_0.firstChild);
-
- // But only move one P!
- break;
+ var block_0_children = block_0.childNodes;
+ if (!block_0_children) {
+ return;
}
-} );
\ No newline at end of file
+
+ var leadSpan = createLeadSpan(block_0_children);
+ block_0.insertBefore(leadSpan, block_0.firstChild)
+} );
+
+// Create a lead span to be moved to the top of the page, consisting of the
first
+// qualifying <p> element encountered and any subsequent non-<p> elements until
+// the next <p> is encountered.
+//
+// Simply moving the first <p> element up may result in elements appearing
+// between the first paragraph as designated by <p></p> tags and other elements
+// (such as an unnumbered list) that may also be intended as part of the first
+// display paragraph. See T111958.
+function createLeadSpan( childNodes ) {
+ var leadSpan = document.createElement( 'span' );
+ var firstParagraphIndex = childNodes.length;
+
+ for (var i = 0; i < childNodes.length; i++) {
+ if (childNodes[i].tagName === 'P') {
+ // Ensure the P being pulled up has at least a couple lines of
text.
+ // Otherwise silly things like a empty P or P which only contains a
+ // BR tag will get pulled up (see articles on "Chemical Reaction"
and
+ // "Hawaii").
+ // Trick for quickly determining element height:
+ //
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement.offsetHeight
+ // http://stackoverflow.com/a/1343350/135557
+ var minHeight = 24;
+ if (childNodes[i].offsetHeight < minHeight){
+ continue;
+ }
+
+
leadSpan.appendChild(childNodes[i].parentNode.removeChild(childNodes[i]));
+ firstParagraphIndex = i;
+ break;
+ }
+ }
+
+ for (var j = firstParagraphIndex + 1; j < childNodes.length; j++) {
+ if (childNodes[j].tagName === 'P') {
+ break;
+ }
+
leadSpan.appendChild(childNodes[j].parentNode.removeChild(childNodes[j]));
+ }
+
+ return leadSpan;
+}
--
To view, visit https://gerrit.wikimedia.org/r/245576
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I41f2b16f9b5d8fcac7bef156a3f3726d7bfe36d0
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
