jenkins-bot has submitted this change and it was merged.
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, 129 insertions(+), 58 deletions(-)
Approvals:
Sniedzielski: Looks good to me, approved
jenkins-bot: Verified
diff --git a/app/src/main/assets/bundle.js b/app/src/main/assets/bundle.js
index 6c994b1..d587347 100644
--- a/app/src/main/assets/bundle.js
+++ b/app/src/main/assets/bundle.js
@@ -1024,51 +1024,87 @@
},{"../transformer":12,"./collapseTables":16}],20:[function(require,module,exports){
var transformer = require("../transformer");
-// 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() {
- if (window.isMainPage) {
+ if ( window.isMainPage ) {
// don't do anything if this is the main page, since many wikis
// arrange the main page in a series of tables.
return;
}
var block_0 = document.getElementById( "content_block_0" );
- if (!block_0) {
+ if ( !block_0 ) {
return;
}
- var allPs = block_0.getElementsByTagName( "p" );
- if (!allPs) {
+ var block_0_children = block_0.childNodes;
+ if ( !block_0_children ) {
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 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 firstGoodParagraphIndex = findFirstGoodParagraphIn( childNodes );
+
+ if ( firstGoodParagraphIndex ) {
+ addNode( leadSpan, childNodes[ firstGoodParagraphIndex ] );
+ addTrailingNodes(leadSpan, childNodes, firstGoodParagraphIndex + 1 );
+ }
+
+ return leadSpan;
+}
+
+function findFirstGoodParagraphIn( nodes ) {
+ var minParagraphHeight = 24;
+ var firstGoodParagraphIndex;
+ var i;
+
+ for ( i = 0; i < nodes.length; i++ ) {
+ if ( nodes[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
+ if ( nodes[i].offsetHeight < minParagraphHeight ){
+ continue;
+ }
+ firstGoodParagraphIndex = i;
+ break;
+ }
+ }
+
+ return firstGoodParagraphIndex;
+}
+
+function addNode( span, node ) {
+ span.appendChild( node.parentNode.removeChild( node ) );
+}
+
+function addTrailingNodes( span, nodes, startIndex ) {
+ for ( var i = startIndex; i < nodes.length; i++ ) {
+ if ( nodes[i].tagName === 'P' ) {
+ break;
+ }
+ addNode( span, nodes[i] );
+ }
+}
+
},{"../transformer":12}],21:[function(require,module,exports){
var transformer = require("../transformer");
diff --git a/www/js/transforms/relocateFirstParagraph.js
b/www/js/transforms/relocateFirstParagraph.js
index 8b0f9e5..f3f3eaf 100644
--- a/www/js/transforms/relocateFirstParagraph.js
+++ b/www/js/transforms/relocateFirstParagraph.js
@@ -1,47 +1,82 @@
var transformer = require("../transformer");
-// 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() {
- if (window.isMainPage) {
+ if ( window.isMainPage ) {
// don't do anything if this is the main page, since many wikis
// arrange the main page in a series of tables.
return;
}
var block_0 = document.getElementById( "content_block_0" );
- if (!block_0) {
+ if ( !block_0 ) {
return;
}
- var allPs = block_0.getElementsByTagName( "p" );
- if (!allPs) {
+ var block_0_children = block_0.childNodes;
+ if ( !block_0_children ) {
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;
- }
+ var leadSpan = createLeadSpan(block_0_children);
+ block_0.insertBefore( leadSpan, block_0.firstChild );
+} );
- // Move the P!
- block_0.insertBefore(p.parentNode.removeChild(p), 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 firstGoodParagraphIndex = findFirstGoodParagraphIn( childNodes );
- // But only move one P!
- break;
+ if ( firstGoodParagraphIndex ) {
+ addNode( leadSpan, childNodes[ firstGoodParagraphIndex ] );
+ addTrailingNodes(leadSpan, childNodes, firstGoodParagraphIndex + 1 );
}
-} );
\ No newline at end of file
+
+ return leadSpan;
+}
+
+function findFirstGoodParagraphIn( nodes ) {
+ var minParagraphHeight = 24;
+ var firstGoodParagraphIndex;
+ var i;
+
+ for ( i = 0; i < nodes.length; i++ ) {
+ if ( nodes[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
+ if ( nodes[i].offsetHeight < minParagraphHeight ){
+ continue;
+ }
+ firstGoodParagraphIndex = i;
+ break;
+ }
+ }
+
+ return firstGoodParagraphIndex;
+}
+
+function addNode( span, node ) {
+ span.appendChild( node.parentNode.removeChild( node ) );
+}
+
+function addTrailingNodes( span, nodes, startIndex ) {
+ for ( var i = startIndex; i < nodes.length; i++ ) {
+ if ( nodes[i].tagName === 'P' ) {
+ break;
+ }
+ addNode( span, nodes[i] );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/245576
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I41f2b16f9b5d8fcac7bef156a3f3726d7bfe36d0
Gerrit-PatchSet: 4
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: Dbrant <[email protected]>
Gerrit-Reviewer: Mholloway <[email protected]>
Gerrit-Reviewer: Mhurd <[email protected]>
Gerrit-Reviewer: Niedzielski <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
