From eb1fe7a8fb0526f12db6aa92a1056518dc8923a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Tue, 22 Feb 2022 10:36:45 +0100 Subject: [PATCH] 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 --- includes/CommentParser.php | 6 ++++-- includes/HeadingItem.php | 6 +++--- modules/HeadingItem.js | 8 ++++---- modules/Parser.js | 6 ++++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/includes/CommentParser.php b/includes/CommentParser.php index 98f32bcc5..15846ce35 100644 --- a/includes/CommentParser.php +++ b/includes/CommentParser.php @@ -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']; + // , or 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']; + // , or in Parsoid HTML + $headline = $threadItemParent->getRange()->startContainer; $id .= '-' . $this->truncateForId( $headline->getAttribute( 'id' ) ?? '' ); } elseif ( $threadItemParent instanceof CommentItem ) { $id .= '-' . $this->truncateForId( str_replace( ' ', '_', $threadItemParent->getAuthor() ) ) . diff --git a/includes/HeadingItem.php b/includes/HeadingItem.php index edf7ea970..28454f092 100644 --- a/includes/HeadingItem.php +++ b/includes/HeadingItem.php @@ -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' ); + // , or 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' ], diff --git a/modules/HeadingItem.js b/modules/HeadingItem.js index 797e91d0a..cb4e9b57e 100644 --- a/modules/HeadingItem.js +++ b/modules/HeadingItem.js @@ -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' ); + // , or 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' ], diff --git a/modules/Parser.js b/modules/Parser.js index 3349d34a0..c87306c24 100644 --- a/modules/Parser.js +++ b/modules/Parser.js @@ -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; + // , or 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; + // , or 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();