CommentParser: Fix redundant uses of getHeadlineNodeAndOffset()

We call CommentUtils::getHeadlineNodeAndOffset() before constructing
the HeadingItem in CommentParser, so the range's startContainer
is always the headline node.

Change-Id: I2afb6ba9100e785cd91f31d82f4cea59fa8b5443
This commit is contained in:
Bartosz Dziewoński 2022-02-22 10:36:45 +01:00
parent 0e576216b2
commit eb1fe7a8fb
4 changed files with 15 additions and 11 deletions

View file

@ -928,7 +928,8 @@ class CommentParser {
// The range points to the root note, using it like below results in silly values
$id = 'h-';
} elseif ( $threadItem instanceof HeadingItem ) {
$headline = CommentUtils::getHeadlineNodeAndOffset( $threadItem->getRange()->startContainer )['node'];
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
$headline = $threadItem->getRange()->startContainer;
$id = 'h-' . $this->truncateForId( $headline->getAttribute( 'id' ) ?? '' );
} elseif ( $threadItem instanceof CommentItem ) {
$id = 'c-' . $this->truncateForId( str_replace( ' ', '_', $threadItem->getAuthor() ) ) .
@ -941,7 +942,8 @@ class CommentParser {
// in one edit, or within a minute), add the parent ID to disambiguate them.
$threadItemParent = $threadItem->getParent();
if ( $threadItemParent instanceof HeadingItem && !$threadItemParent->isPlaceholderHeading() ) {
$headline = CommentUtils::getHeadlineNodeAndOffset( $threadItemParent->getRange()->startContainer )['node'];
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
$headline = $threadItemParent->getRange()->startContainer;
$id .= '-' . $this->truncateForId( $headline->getAttribute( 'id' ) ?? '' );
} elseif ( $threadItemParent instanceof CommentItem ) {
$id .= '-' . $this->truncateForId( str_replace( ' ', '_', $threadItemParent->getAuthor() ) ) .

View file

@ -38,9 +38,9 @@ class HeadingItem extends ThreadItem {
$title = '';
// If this comment is in 0th section, there's no section title for the edit summary
if ( !$this->isPlaceholderHeading() ) {
$headingNode =
CommentUtils::getHeadlineNodeAndOffset( $this->getRange()->startContainer )['node'];
$id = $headingNode->getAttribute( 'id' );
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
$headline = $this->getRange()->startContainer;
$id = $headline->getAttribute( 'id' );
if ( $id ) {
// Replace underscores with spaces to undo Sanitizer::escapeIdInternal().
// This assumes that $wgFragmentMode is [ 'html5', 'legacy' ] or [ 'html5' ],

View file

@ -1,5 +1,4 @@
var ThreadItem = require( './ThreadItem.js' ),
utils = require( './utils.js' );
var ThreadItem = require( './ThreadItem.js' );
/**
* A heading item
@ -25,8 +24,9 @@ HeadingItem.prototype.getLinkableTitle = function () {
var title = '';
// If this comment is in 0th section, there's no section title for the edit summary
if ( !this.placeholderHeading ) {
var headingNode = utils.getHeadlineNodeAndOffset( this.range.startContainer ).node;
var id = headingNode.getAttribute( 'id' );
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
var headline = this.range.startContainer;
var id = headline.getAttribute( 'id' );
if ( id ) {
// Replace underscores with spaces to undo Sanitizer::escapeIdInternal().
// This assumes that $wgFragmentMode is [ 'html5', 'legacy' ] or [ 'html5' ],

View file

@ -914,7 +914,8 @@ Parser.prototype.computeId = function ( threadItem, previousItems ) {
// The range points to the root note, using it like below results in silly values
id = 'h-';
} else if ( threadItem instanceof HeadingItem ) {
headline = utils.getHeadlineNodeAndOffset( threadItem.range.startContainer ).node;
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
headline = threadItem.range.startContainer;
id = 'h-' + this.truncateForId( headline.getAttribute( 'id' ) || '' );
} else if ( threadItem instanceof CommentItem ) {
id = 'c-' + this.truncateForId( threadItem.author || '' ).replace( / /g, '_' ) + '-' + threadItem.timestamp.toISOString();
@ -926,7 +927,8 @@ Parser.prototype.computeId = function ( threadItem, previousItems ) {
// in one edit, or within a minute), append sequential numbers
var threadItemParent = threadItem.parent;
if ( threadItemParent instanceof HeadingItem && !threadItemParent.placeholderHeading ) {
headline = utils.getHeadlineNodeAndOffset( threadItemParent.range.startContainer ).node;
// <span class="mw-headline" …>, or <hN …> in Parsoid HTML
headline = threadItemParent.range.startContainer;
id += '-' + this.truncateForId( headline.getAttribute( 'id' ) || '' );
} else if ( threadItemParent instanceof CommentItem ) {
id += '-' + this.truncateForId( threadItemParent.author || '' ).replace( / /g, '_' ) + '-' + threadItemParent.timestamp.toISOString();