From bc47b4fb3eb1660586e2b27e41448aa7b8138662 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 8 Nov 2017 21:41:12 -0800
Subject: [PATCH] VectorTemplate: Refactor literal HTML by using Html methods
 instead

To some degree the literal HTML was (maybe) useful and self-documenting
at some point when the template was really simple, but until and unless
we really use an Html template for this, it's probably a lot easier to
maintain, understand and review (incl. from security perspective) if
we consistently use the Html class abstraction.

For now, I'm only focussing on cases where there is mixed literal HTML
with embedded PHP statements. The cases where HTML is created plain without
embedded PHP I'm leaving untouched for now.

Any case where attribute or content comes from PHP, use the Html class
instead to clearly indicate which values are escaped, and which are not.

Change-Id: Ib2d6425994918b0c17ef29c1b5d0f9893f61a889
---
 VectorTemplate.php | 56 ++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/VectorTemplate.php b/VectorTemplate.php
index 031e6f0..110d902 100644
--- a/VectorTemplate.php
+++ b/VectorTemplate.php
@@ -61,30 +61,40 @@ class VectorTemplate extends BaseTemplate {
 			<a id="top"></a>
 			<?php
 			if ( $this->data['sitenotice'] ) {
-				?>
-				<div id="siteNotice" class="mw-body-content"><?php $this->html( 'sitenotice' ) ?></div>
-			<?php
+				echo Html::rawElement( 'div',
+					[ 'class' => 'mw-body-content' ],
+					// Raw HTML
+					$this->get( 'sitenotice' )
+				);
 			}
-			?>
-			<?php
 			if ( is_callable( [ $this, 'getIndicators' ] ) ) {
 				echo $this->getIndicators();
 			}
 			// Loose comparison with '!=' is intentional, to catch null and false too, but not '0'
 			if ( $this->data['title'] != '' ) {
+				echo Html::rawElement( 'h1',
+					[
+						'id' => 'firstHeading',
+						'class' => 'firstHeading',
+						'lang' => $this->get( 'pageLanguage' ),
+					],
+					// Raw HTML
+					$this->get( 'title' )
+				);
+			}
+
+			$this->html( 'prebodyhtml' );
 			?>
-			<h1 id="firstHeading" class="firstHeading" lang="<?php $this->text( 'pageLanguage' ); ?>"><?php
-				$this->html( 'title' )
-			?></h1>
-			<?php
-			} ?>
-			<?php $this->html( 'prebodyhtml' ) ?>
 			<div id="bodyContent" class="mw-body-content">
 				<?php
 				if ( $this->data['isarticle'] ) {
-					?>
-					<div id="siteSub" class="noprint"><?php $this->msg( 'tagline' ) ?></div>
-				<?php
+					echo Html::element( 'div',
+						[
+							'id' => 'siteSub',
+							'class' => 'noprint',
+						],
+						$this->getMsg( 'tagline' )->text()
+					);
 				}
 				?>
 				<div id="contentSub"<?php $this->html( 'userlangattributes' ) ?>><?php
@@ -92,16 +102,18 @@ class VectorTemplate extends BaseTemplate {
 				?></div>
 				<?php
 				if ( $this->data['undelete'] ) {
-					?>
-					<div id="contentSub2"><?php $this->html( 'undelete' ) ?></div>
-				<?php
+					echo Html::rawElement( 'div',
+						[ 'id' => 'contentSub2' ],
+						// Raw HTML
+						$this->get( 'undelete' )
+					);
 				}
-				?>
-				<?php
 				if ( $this->data['newtalk'] ) {
-					?>
-					<div class="usermessage"><?php $this->html( 'newtalk' ) ?></div>
-				<?php
+					echo Html::rawElement( 'div',
+						[ 'class' => 'usermessage' ],
+						// Raw HTML
+						$this->get( 'newtalk' )
+					);
 				}
 				?>
 				<div id="jump-to-nav" class="mw-jump">