From 82a2edf5857adba639af223345de3d08d7e07092 Mon Sep 17 00:00:00 2001
From: Piotr Miazga <piotr@polishdeveloper.pl>
Date: Thu, 23 May 2019 21:10:51 +0200
Subject: [PATCH] Untangle watchstar icon

The SkinMinerva::createWatchPageAction() was depending upon
$tpl['content_navigation']['actions'] object, which is created by
SkinTemplate::buildContentNavigationUrls(). Although only 'watch'
and 'unwatch' keys were used, moreover, code was using only 'href'
property from both 'watch' and 'unwatch'. All other props are
overrided, not used when rendering. Also the title of unwatch
icon was set to "watch this page" which was incorrect.

This system was bit complex to manage (we should provide dependencies
not pass the whole 'content_navigation']['actions'] array). It would
also make further refactoring more difficult, as watchstar icon
shouldn't depened on quicktemplate stuff. We were using only href
attribute which is super easy to calculate.

Changes:
 - do not depend upon $actions, get href by using $title->getLocalUrl
 - set proper title for unwatch icon
 - simplified SkinMinerva::createWatchPageAction() logic

Bug: T221792
Change-Id: I9609949b60a7e2f2f0a947c05c80c89be32d4cb1
---
 includes/skins/SkinMinerva.php | 47 ++++++++++++++--------------------
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index dd26a9e84..33b793f9c 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -845,11 +845,7 @@ class SkinMinerva extends SkinTemplate {
 		}
 
 		if ( $this->isAllowedPageAction( 'watch' ) ) {
-			// SkinTemplate#buildContentNavigationUrls creates distinct "watch" and "unwatch" actions.
-			// Pass these actions in as context for #createWatchPageAction.
-			$actions = $tpl->data['content_navigation']['actions'];
-
-			$toolbar[] = $this->createWatchPageAction( $actions );
+			$toolbar[] = $this->createWatchPageAction();
 		}
 
 		if ( $this->isAllowedPageAction( 'history' ) ) {
@@ -907,42 +903,37 @@ class SkinMinerva extends SkinTemplate {
 	 * add the page to or remove the page from the user's watchlist; or, if the user is logged out,
 	 * will direct the user's UA to Special:Login.
 	 *
-	 * @param array $actions
 	 * @return array A map of HTML attributes and a "text" property to be used with the
 	 * pageActionMenu.mustache template.
 	 */
-	protected function createWatchPageAction( $actions ) {
+	protected function createWatchPageAction() {
 		$title = $this->getTitle();
 		$user = $this->getUser();
-		$ctaUrl = $this->getLoginUrl( [ 'returnto' => $title ] );
-		if ( $title && $user->isWatched( $title ) ) {
+		$isWatched = $title && $user->isLoggedIn() && $user->isWatched( $title );
+		$actionOnClick = $isWatched ? 'unwatch' : 'watch';
+		$href = $user->isAnon()
+			? $this->getLoginUrl( [ 'returnto' => $title ] )
+			: $title->getLocalURL( [ 'action' => $actionOnClick ] );
+		$additionalClassNames = ' jsonly';
+
+		if ( $isWatched ) {
+			$msg = $this->msg( 'unwatchthispage' );
 			$icon = 'watched';
+			$additionalClassNames .= ' watched';
 		} else {
+			$msg = $this->msg( 'watchthispage' );
 			$icon = 'watch';
 		}
-		$baseResult = [
+		return [
 			'item-id' => 'page-actions-watch',
 			'id' => 'ca-watch',
 			// Use blank icon to reserve space for watchstar icon once JS loads
-			'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' ) . ' jsonly',
-			'title' => $this->msg( 'watchthispage' ),
-			'text' => $this->msg( 'watchthispage' )
+			'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' )
+				. $additionalClassNames,
+			'title' => $msg,
+			'text' => $msg,
+			'href' => $href
 		];
-
-		if ( isset( $actions['watch'] ) ) {
-			$result = array_merge( $actions['watch'], $baseResult );
-		} elseif ( isset( $actions['unwatch'] ) ) {
-			$result = array_merge( $actions['unwatch'], $baseResult );
-			$result['class'] .= ' watched';
-			$result[ 'text' ] = $this->msg( 'unwatchthispage' );
-		} else {
-			// placeholder for not logged in
-			$result = array_merge( $baseResult, [
-				'href' => $ctaUrl,
-			] );
-		}
-
-		return $result;
 	}
 
 	/**