From 7abd4621aebc910040d587bc464130e896363fd5 Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Tue, 27 Oct 2020 12:18:50 +0000 Subject: [PATCH] Use new heading markers for getting heading ranges/text This will need to be deployed a few days after I782caafc3 to avoid issues with cached HTML. Change-Id: I4b00d0b0efed9d91c0bc28914bbdf0955bb552b6 --- modules/ThreadItem.js | 28 +++++++++------------------- modules/controller.js | 2 +- modules/dt.ui.ReplyWidget.js | 10 +++------- 3 files changed, 13 insertions(+), 27 deletions(-) diff --git a/modules/ThreadItem.js b/modules/ThreadItem.js index d54a8bb76..430d2a2b9 100644 --- a/modules/ThreadItem.js +++ b/modules/ThreadItem.js @@ -47,17 +47,17 @@ OO.initClass( ThreadItem ); * * @param {string|Object} json JSON serialization or hash object * @param {Object} commentsById Collection of comments by ID for building replies/parent pointers - * @param {Node} placeholderNode Placeholder node in the DOM which contained this JSON * @return {ThreadItem} * @throws {Error} Unknown ThreadItem type */ -ThreadItem.static.newFromJSON = function ( json, commentsById, placeholderNode ) { +ThreadItem.static.newFromJSON = function ( json, commentsById ) { // The page can be served from the HTTP cache (Varnish), and the JSON may be generated // by an older version of our PHP code. Code below must be able to handle that. // See ThreadItem::jsonSerialize() in PHP. var CommentItem, HeadingItem, item, idEscaped, hash = typeof json === 'string' ? JSON.parse( json ) : json; + switch ( hash.type ) { case 'comment': // Late require to avoid circular dependency @@ -81,25 +81,15 @@ ThreadItem.static.newFromJSON = function ( json, commentsById, placeholderNode ) default: throw new Error( 'Unknown ThreadItem type ' + hash.name ); } - item.id = hash.id; - if ( hash.type === 'comment' ) { - idEscaped = $.escapeSelector( item.id ); - item.range = { - startContainer: document.querySelector( '[data-mw-comment-start="' + idEscaped + '"]' ), - startOffset: 0, - endContainer: document.querySelector( '[data-mw-comment-end="' + idEscaped + '"]' ), - endOffset: 0 - }; - } else { - item.range = { - startContainer: placeholderNode, - startOffset: 0, - endContainer: placeholderNode, - endOffset: 0 - }; - } + idEscaped = $.escapeSelector( item.id ); + item.range = { + startContainer: document.querySelector( '[data-mw-comment-start="' + idEscaped + '"]' ), + startOffset: 0, + endContainer: document.querySelector( '[data-mw-comment-end="' + idEscaped + '"]' ), + endOffset: 0 + }; // Setup replies/parent pointers item.replies = hash.replies.map( function ( id ) { diff --git a/modules/controller.js b/modules/controller.js index 44dad0e37..50fd9311f 100644 --- a/modules/controller.js +++ b/modules/controller.js @@ -226,7 +226,7 @@ function init( $container, state ) { // Iterate over commentNodes backwards so replies are always deserialized before their parents. for ( i = commentNodes.length - 1; i >= 0; i-- ) { hash = JSON.parse( commentNodes[ i ].getAttribute( 'data-mw-comment' ) ); - comment = ThreadItem.static.newFromJSON( hash, threadItemsById, commentNodes[ i ] ); + comment = ThreadItem.static.newFromJSON( hash, threadItemsById ); threadItemsById[ comment.id ] = comment; if ( comment.type === 'comment' ) { diff --git a/modules/dt.ui.ReplyWidget.js b/modules/dt.ui.ReplyWidget.js index 37b52fa90..e78b9f794 100644 --- a/modules/dt.ui.ReplyWidget.js +++ b/modules/dt.ui.ReplyWidget.js @@ -1,8 +1,7 @@ var controller = require( 'ext.discussionTools.init' ).controller, modifier = require( 'ext.discussionTools.init' ).modifier, utils = require( 'ext.discussionTools.init' ).utils, - logger = require( 'ext.discussionTools.init' ).logger, - Parser = require( 'ext.discussionTools.init' ).Parser; + logger = require( 'ext.discussionTools.init' ).logger; /** * @external CommentController @@ -377,7 +376,7 @@ ReplyWidget.prototype.onModeTabSelectChoose = function ( option ) { * @return {ReplyWidget} */ ReplyWidget.prototype.setup = function ( data ) { - var heading, headingNode, summary; + var heading, summary; data = data || {}; @@ -395,10 +394,7 @@ ReplyWidget.prototype.setup = function ( data ) { // This comment is in 0th section, there's no section title for the edit summary summary = ''; } else { - headingNode = ( new Parser() ).getHeadlineNodeAndOffset( heading.range.startContainer ).node; - // Remove mw-headline-number. T264561 - $( headingNode ).find( '.mw-headline-number' ).remove(); - summary = '/* ' + headingNode.innerText.trim() + ' */ '; + summary = '/* ' + heading.getNativeRange().toString().trim() + ' */ '; } summary += mw.msg( 'discussiontools-defaultsummary-reply' ); }