CR fixes, PLATFORM-3904

thamcheongkit · Feb 8, 2019 · c0944b4 · c0944b4
1 parent c8cc168
commit c0944b4
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 17 deletions.
diff --git a/extensions/wikia/SEOTweaks/SEOTweaks.setup.php b/extensions/wikia/SEOTweaks/SEOTweaks.setup.php
@@ -22,7 +22,6 @@
 $wgHooks['LinkEnd'][] = 'SEOTweaksHooksHelper::onLinkEnd';
 $wgHooks['OpenGraphMetaHeaders'][] = 'SEOTweaksHooksHelper::onOpenGraphMetaHeaders';
 $wgHooks['ShowMissingArticle'][] = 'SEOTweaksHooksHelper::onShowMissingArticle';
-$wgHooks['ArticleViewRedirect'][] = 'SEOTweaksHooksHelper::onArticleViewRedirect';
 
 
 // messages

diff --git a/extensions/wikia/SEOTweaks/SEOTweaksHooksHelper.class.php b/extensions/wikia/SEOTweaks/SEOTweaksHooksHelper.class.php
@@ -359,20 +359,4 @@ public static function onLinkEnd( $skin, Title $target, array $options, &$text,
 	}
 
 
-	/**
-	 * Hook: set script that changes the address bar in the browser for the redirected url
-	 * @param Article $page
-	 * @return bool
-	 */
-	public static function onArticleViewRedirect( Article $page ): bool {
-		if( !$page->getContext()->getUser()->isAnon() ) {
-			$script = 'history.replaceState({}, "' . $page->getTitle()->getText().'", "' .
-					  $page->getTitle()->getLocalURL() . '");';
-			$page->getContext()->getOutput()->addInlineScript( $script );
-		}
-
-		return true;
-	}
-
-
 }
diff --git a/includes/Article.php b/includes/Article.php
@@ -930,6 +930,18 @@ public function showRedirectedFromHeader() {
 					$fragment = Xml::escapeJsString( $this->getTitle()->getFragmentForURL() );
 					$wgOut->addInlineScript( "redirectToFragment(\"$fragment\");" );
 				}
+
+				if( !$this->getContext()->getUser()->isAnon() ) {
+					// Add the script to update the displayed URL and
+					// set the fragment if one was specified in the redirect
+					$wgOut->addJsConfigVars( [
+						'wgInternalRedirectTargetUrl' => $this->getTitle()->getLocalURL(),
+					] );
+					$wgOut->addModules( [ 'mediawiki.action.view.redirect' ] );
+				}
+
+
+
 				/**
 				 * Commented out by christian@wikia-inc.com
 				 * /extensions/wikia/CanonicalHref is used by Wikia to handle redirects and all of these cases:

diff --git a/resources/Resources.php b/resources/Resources.php
@@ -578,6 +578,13 @@
 			'jquery.byteLimit',
 		),
 	),
+	'mediawiki.action.view.redirect' => array(
+		'scripts' => 'resources/mediawiki.action/mediawiki.action.view.redirect.js',
+		'dependencies' => array(
+			'jquery.client',
+		),
+		'position' => 'top',
+	),
 	'mediawiki.action.history' => array(
 		'scripts' => 'resources/mediawiki.action/mediawiki.action.history.js',
 		'dependencies' => 'jquery.ui.button',

diff --git a/resources/mediawiki.action/mediawiki.action.view.redirect.js b/resources/mediawiki.action/mediawiki.action.view.redirect.js
@@ -0,0 +1,65 @@
+/*!
+ * JavaScript to update page URL when a redirect is viewed, ensuring that the
+ * page is scrolled to the id when it's a redirect with fragment.
+ *
+ * This is loaded in the top queue, so avoid unnecessary dependencies
+ * like mediawiki.Title or mediawiki.Uri.
+ */
+( function () {
+	var profile = $.client.profile(),
+		canonical = mw.config.get( 'wgInternalRedirectTargetUrl' ),
+		fragment = null,
+		node, shouldChangeFragment, index;
+
+	index = canonical.indexOf( '#' );
+	if ( index !== -1 ) {
+		fragment = canonical.slice( index );
+	}
+
+	// Never override the fragment if the user intended to look at a different section
+	shouldChangeFragment = fragment && !location.hash;
+
+	// Replace the whole URL if possible, otherwise just change the fragment
+	if ( canonical && history.replaceState ) {
+		if ( !shouldChangeFragment ) {
+			// If the current page view has a fragment already, don't override it
+			canonical = canonical.replace( /#.*$/, '' );
+			canonical += location.hash;
+		}
+
+		// Note that this will update the hash in a modern browser, retaining back behaviour
+		history.replaceState( /* data= */ history.state, /* title= */ document.title, /* url= */ canonical );
+		if ( shouldChangeFragment ) {
+			// Specification for history.replaceState() doesn't require browser to scroll,
+			// so scroll to be sure (see also T110501). Support for IE10.
+			node = document.getElementById( fragment.slice( 1 ) );
+			if ( node ) {
+				node.scrollIntoView();
+			}
+		}
+
+	} else if ( shouldChangeFragment ) {
+		if ( profile.layout === 'webkit' && profile.layoutVersion < 420 ) {
+			// Released Safari w/ WebKit 418.9.1 messes up horribly
+			// Nightlies of 420+ are ok
+			return;
+		}
+
+		location.hash = fragment;
+	}
+
+	if ( shouldChangeFragment && profile.layout === 'gecko' ) {
+		// Mozilla needs to wait until after load, otherwise the window doesn't
+		// scroll.  See <https://bugzilla.mozilla.org/show_bug.cgi?id=516293>.
+		// There's no obvious way to detect this programmatically, so we use
+		// version-testing.  If Firefox fixes the bug, they'll jump twice, but
+		// better twice than not at all, so make the fix hit future versions as
+		// well.
+		$( function () {
+			if ( location.hash === fragment ) {
+				location.hash = fragment;
+			}
+		} );
+	}
+
+}() );