Rename ParserOutput::{allow,prevent}Clickjacking() -> ::{get,set}PreventClickjacking()

This name is consist with the rest of the setter and getter methods
in ParserOutput.  Renamed the methods in OutputPage, ImageHistoryList,
ImageHistoryPseudoPager, and ContribsPager as well for consistency;
it also makes chasing down lingering references in codesearch easier.

Soft-deprecated the old name for 1.38.  Hard-deprecation will follow,
but there are a number of users in production that should be chased
down first.

Code search:

https://codesearch.https://codesearch.wmcloud.org/deployed/?q=(allow%7Cprevent)Clickjacking&i=nope&files=&excludeFiles=&repos=

Bug: T287216
Change-Id: I9822c60c180d204bd30cb4447a1120155d456da4

RELEASE-NOTES-1.38

@@ -167,6 +167,14 @@ because of Phabricator reports.
 * Message::content() was hard-deprecated.
 * The following methods from the ParserOutput class were hard deprecated:
    - ::hideNewSection() - use ::setHideNewSection()
+   - ::preventClickjacking() - use ::{get,set}PreventClickjacking()
+* The following methods were soft-deprecated; use ::setPreventClickjacking(..)
+  instead:
+   - OutputPage::preventClickjacking()
+   - OutputPage::allowClickjacking()
+   - ImageHistoryList::preventClickjacking()
+   - ImageHistoryPseudoPager::preventClickjacking()
+   - ContribsPager::preventClickjacking()
 * Collation::singleton() and ::factory() were hard-deprecated.
 * …
 

includes/OutputPage.php

@@ -254,7 +254,7 @@ class OutputPage extends ContextSource {
 	/**
 	 * @var bool Controls if anti-clickjacking / frame-breaking headers will
 	 * be sent. This should be done for pages where edit actions are possible.
-	 * Setters: $this->preventClickjacking() and $this->allowClickjacking().
+	 * Setter: $this->setPreventClickjacking()
 	 */
 	protected $mPreventClickjacking = true;
 
@@ -1905,7 +1905,7 @@ class OutputPage extends ContextSource {
 		$this->addModuleStyles( $parserOutput->getModuleStyles() );
 		$this->addJsConfigVars( $parserOutput->getJsConfigVars() );
 		$this->mPreventClickjacking = $this->mPreventClickjacking
-			|| $parserOutput->preventClickjacking();
+			|| $parserOutput->getPreventClickjacking();
 		$scriptSrcs = $parserOutput->getExtraCSPScriptSrcs();
 		foreach ( $scriptSrcs as $src ) {
 			$this->getCSP()->addScriptSrc( $src );
@@ -2351,6 +2351,7 @@ class OutputPage extends ContextSource {
 	 * form on an ordinary view page, then you need to call this function.
 	 *
 	 * @param bool $enable
+	 * @deprecated since 1.38, use ::setPreventClickjacking( true )
 	 */
 	public function preventClickjacking( $enable = true ) {
 		$this->mPreventClickjacking = $enable;
@@ -2360,11 +2361,35 @@ class OutputPage extends ContextSource {
 	 * Turn off frame-breaking. Alias for $this->preventClickjacking(false).
 	 * This can be called from pages which do not contain any CSRF-protected
 	 * HTML form.
+	 *
+	 * @deprecated since 1.38, use ::setPreventClickjacking( false )
 	 */
 	public function allowClickjacking() {
 		$this->mPreventClickjacking = false;
 	}
 
+	/**
+	 * Set the prevent-clickjacking flag.
+	 *
+	 * If true, will cause an X-Frame-Options header appropriate for
+	 * edit pages to be sent. The header value is controlled by
+	 * $wgEditPageFrameOptions.  This is the default for special
+	 * pages. If you display a CSRF-protected form on an ordinary view
+	 * page, then you need to call this function.
+	 *
+	 * Setting this flag to false will turn off frame-breaking.  This
+	 * can be called from pages which do not contain any
+	 * CSRF-protected HTML form.
+	 *
+	 * @param bool $enable If true, will cause an X-Frame-Options header
+	 *  appropriate for edit pages to be sent.
+	 *
+	 * @since 1.38
+	 */
+	public function setPreventClickjacking( bool $enable ) {
+		$this->mPreventClickjacking = $enable;
+	}
+
 	/**
 	 * Get the prevent-clickjacking flag
 	 *

includes/actions/HistoryAction.php

@@ -320,7 +320,7 @@ class HistoryAction extends FormlessAction {
 			$pager->getBody() .
 			$pager->getNavigationBar()
 		);
-		$out->preventClickjacking( $pager->getPreventClickjacking() );
+		$out->setPreventClickjacking( $pager->getPreventClickjacking() );
 
 		return null;
 	}

includes/actions/pagers/HistoryPager.php

@@ -290,7 +290,7 @@ class HistoryPager extends ReverseChronologicalPager {
 	}
 
 	private function getRevisionButton( $name, $msg ) {
-		$this->preventClickjacking();
+		$this->setPreventClickjacking( true );
 		$element = Html::element(
 			'button',
 			[
@@ -408,7 +408,7 @@ class HistoryPager extends ReverseChronologicalPager {
 		// change tags
 		$visibility = $revRecord->getVisibility();
 		if ( $canRevDelete || $this->showTagEditUI ) {
-			$this->preventClickjacking();
+			$this->setPreventClickjacking( true );
 			// If revision was hidden from sysops and we don't need the checkbox
 			// for anything else, disable it
 			if ( !$this->showTagEditUI
@@ -487,7 +487,7 @@ class HistoryPager extends ReverseChronologicalPager {
 					[ 'verify', 'noBrackets' ]
 				);
 				if ( $rollbackLink ) {
-					$this->preventClickjacking();
+					$this->setPreventClickjacking( true );
 					$tools[] = $rollbackLink;
 				}
 			}
@@ -723,11 +723,12 @@ class HistoryPager extends ReverseChronologicalPager {
 	}
 
 	/**
-	 * This is called if a write operation is possible from the generated HTML
-	 * @param bool $enable
+	 * Set the "prevent clickjacking" flag; for example if a write operation
+	 * if possible from the generated HTML.
+	 * @param bool $flag
 	 */
-	private function preventClickjacking( $enable = true ) {
-		$this->preventClickjacking = $enable;
+	private function setPreventClickjacking( bool $flag ) {
+		$this->preventClickjacking = $flag;
 	}
 
 	/**

includes/api/ApiFormatBase.php

@@ -329,7 +329,7 @@ abstract class ApiFormatBase extends ApiBase {
 			} else {
 				// API handles its own clickjacking protection.
 				// Note, that $wgBreakFrames will still override $wgApiFrameOptions for format mode.
-				$out->allowClickjacking();
+				$out->setPreventClickjacking( false );
 				$out->output();
 			}
 		} else {

includes/diff/DifferenceEngine.php

@@ -606,7 +606,7 @@ class DifferenceEngine extends ContextSource {
 	public function showDiffPage( $diffOnly = false ) {
 		# Allow frames except in certain special cases
 		$out = $this->getOutput();
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 		$out->setRobotPolicy( 'noindex,nofollow' );
 
 		// Allow extensions to add any extra output here
@@ -685,7 +685,7 @@ class DifferenceEngine extends ContextSource {
 						[ 'noBrackets' ]
 					);
 					if ( $rollbackLink ) {
-						$out->preventClickjacking();
+						$out->setPreventClickjacking( true );
 						$rollback = "\u{00A0}\u{00A0}\u{00A0}" . $rollbackLink;
 					}
 				}
@@ -862,7 +862,7 @@ class DifferenceEngine extends ContextSource {
 	 * Returns empty string if there's either no revision to patrol or the user is not allowed to.
 	 *
 	 * Side effect: When the patrol link is build, this method will call
-	 * OutputPage::preventClickjacking() and load a JS module.
+	 * OutputPage::setPreventClickjacking(true) and load a JS module.
 	 *
 	 * @return string HTML or empty string
 	 */
@@ -937,7 +937,7 @@ class DifferenceEngine extends ContextSource {
 
 			// Build the link
 			if ( $rcid ) {
-				$this->getOutput()->preventClickjacking();
+				$this->getOutput()->setPreventClickjacking( true );
 				if ( $this->getAuthority()->isAllowed( 'writeapi' ) ) {
 					$this->getOutput()->addModules( 'mediawiki.misc-authed-curate' );
 				}

includes/htmlform/HTMLForm.php

@@ -1090,7 +1090,7 @@ class HTMLForm extends ContextSource {
 	 */
 	public function getHTML( $submitResult ) {
 		# For good measure (it is the default)
-		$this->getOutput()->preventClickjacking();
+		$this->getOutput()->setPreventClickjacking( true );
 		$this->getOutput()->addModules( 'mediawiki.htmlform' );
 		$this->getOutput()->addModuleStyles( 'mediawiki.htmlform.styles' );
 

includes/page/Article.php

@@ -465,7 +465,7 @@ class Article implements Page {
 
 		$outputPage->setArticleFlag( true );
 		# Allow frames by default
-		$outputPage->allowClickjacking();
+		$outputPage->setPreventClickjacking( false );
 
 		$parserOptions = $this->getParserOptions();
 		$poOptions = [];
@@ -1108,7 +1108,7 @@ class Article implements Page {
 	 * desired, does nothing.
 	 *
 	 * Side effect: When the patrol link is build, this method will call
-	 * OutputPage::preventClickjacking() and load a JS module.
+	 * OutputPage::setPreventClickjacking(true) and load a JS module.
 	 *
 	 * @return bool
 	 */
@@ -1251,7 +1251,7 @@ class Article implements Page {
 			return false;
 		}
 
-		$outputPage->preventClickjacking();
+		$outputPage->setPreventClickjacking( true );
 		if ( $this->getContext()->getAuthority()->isAllowed( 'writeapi' ) ) {
 			$outputPage->addModules( 'mediawiki.misc-authed-curate' );
 		}

includes/page/ImageHistoryList.php

@@ -206,7 +206,7 @@ class ImageHistoryList extends ContextSource {
 		} elseif ( $file->isDeleted( File::DELETED_FILE ) ) {
 			$timeAndDate = $lang->userTimeAndDate( $timestamp, $user );
 			if ( $local ) {
-				$this->preventClickjacking();
+				$this->setPreventClickjacking( true );
 				$revdel = SpecialPage::getTitleFor( 'Revisiondelete' );
 				# Make a link to review the image
 				$url = $linkRenderer->makeKnownLink(
@@ -324,11 +324,20 @@ class ImageHistoryList extends ContextSource {
 
 	/**
 	 * @param bool $enable
+	 * @deprecated since 1.38, use ::setPreventClickjacking() instead
 	 */
 	protected function preventClickjacking( $enable = true ) {
 		$this->preventClickjacking = $enable;
 	}
 
+	/**
+	 * @param bool $enable
+	 * @since 1.38
+	 */
+	protected function setPreventClickjacking( bool $enable ) {
+		$this->preventClickjacking = $enable;
+	}
+
 	/**
 	 * @return bool
 	 */

includes/page/ImageHistoryPseudoPager.php

@@ -144,7 +144,7 @@ class ImageHistoryPseudoPager extends ReverseChronologicalPager {
 			$s .= $list->endImageHistoryList( $navLink );
 
 			if ( $list->getPreventClickjacking() ) {
-				$this->preventClickjacking();
+				$this->setPreventClickjacking( true );
 			}
 		}
 		return $s;
@@ -241,11 +241,20 @@ class ImageHistoryPseudoPager extends ReverseChronologicalPager {
 
 	/**
 	 * @param bool $enable
+	 * @deprecated since 1.38, use ::setPreventClickjacking()
 	 */
 	protected function preventClickjacking( $enable = true ) {
 		$this->preventClickjacking = $enable;
 	}
 
+	/**
+	 * @param bool $enable
+	 * @since 1.38
+	 */
+	protected function setPreventClickjacking( bool $enable ) {
+		$this->preventClickjacking = $enable;
+	}
+
 	/**
 	 * @return bool
 	 */

includes/page/ImagePage.php

@@ -836,7 +836,7 @@ EOT
 			MediaWikiServices::getInstance()->getLinkBatchFactory()
 		);
 		$out->addHTML( $pager->getBody() );
-		$out->preventClickjacking( $pager->getPreventClickjacking() );
+		$out->setPreventClickjacking( $pager->getPreventClickjacking() );
 
 		$this->getFile()->resetHistory(); // free db resources
 

includes/parser/ParserOutput.php

@@ -1389,15 +1389,41 @@ class ParserOutput extends CacheTime {
 		return $this->hasReducedExpiry();
 	}
 
+	/**
+	 * Set the prevent-clickjacking flag
+	 *
+	 * @param bool $flag New flag value
+	 * @since 1.38
+	 */
+	public function setPreventClickjacking( bool $flag ) {
+		$this->mPreventClickjacking = $flag;
+	}
+
+	/**
+	 * Get the prevent-clickjacking flag
+	 *
+	 * @return bool Flag value
+	 * @since 1.38
+	 */
+	public function getPreventClickjacking(): bool {
+		return $this->mPreventClickjacking;
+	}
+
 	/**
 	 * Get or set the prevent-clickjacking flag
 	 *
 	 * @since 1.24
 	 * @param bool|null $flag New flag value, or null to leave it unchanged
 	 * @return bool Old flag value
+	 * @deprecated since 1.38:
+	 *   use ::setPreventClickjacking() or ::getPreventClickjacking()
 	 */
-	public function preventClickjacking( $flag = null ) {
-		return wfSetVar( $this->mPreventClickjacking, $flag );
+	public function preventClickjacking( $flag ) {
+		$old = $this->getPreventClickjacking();
+		if ( $flag !== null ) {
+			$this->setPreventClickjacking( $flag );
+		}
+		return $old;
 	}
 
 	/**
@@ -1559,7 +1585,7 @@ class ParserOutput extends CacheTime {
 		$this->mHideNewSection = $this->mHideNewSection || $source->getHideNewSection();
 		$this->mNoGallery = $this->mNoGallery || $source->getNoGallery();
 		$this->mEnableOOUI = $this->mEnableOOUI || $source->getEnableOOUI();
-		$this->mPreventClickjacking = $this->mPreventClickjacking || $source->preventClickjacking();
+		$this->mPreventClickjacking = $this->mPreventClickjacking || $source->getPreventClickjacking();
 
 		// TODO: we'll have to be smarter about this!
 		$this->mSections = array_merge( $this->mSections, $source->getSections() );

includes/specials/SpecialAllPages.php

@@ -78,7 +78,7 @@ class SpecialAllPages extends IncludableSpecialPage {
 
 		$this->setHeaders();
 		$this->outputHeader();
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 
 		# GET values
 		$from = $request->getVal( 'from', null );

includes/specials/SpecialCategories.php

@@ -52,7 +52,7 @@ class SpecialCategories extends SpecialPage {
 		$this->setHeaders();
 		$this->outputHeader();
 		$this->addHelpLink( 'Help:Categories' );
-		$this->getOutput()->allowClickjacking();
+		$this->getOutput()->setPreventClickjacking( false );
 
 		$from = $this->getRequest()->getText( 'from', $par );
 

includes/specials/SpecialContributions.php

@@ -349,7 +349,7 @@ class SpecialContributions extends IncludableSpecialPage {
 				$work->execute();
 			}
 
-			$out->preventClickjacking( $pager->getPreventClickjacking() );
+			$out->setPreventClickjacking( $pager->getPreventClickjacking() );
 
 			# Show the appropriate "footer" message - WHOIS tools, etc.
 			if ( IPUtils::isValidRange( $target ) && $pager->isQueryableRange( $target ) ) {

includes/specials/SpecialJavaScriptTest.php

@@ -56,7 +56,7 @@ class SpecialJavaScriptTest extends SpecialPage {
 
 		// Allow framing (disabling wgBreakFrames). Otherwise, mediawiki.page.ready
 		// will close this tab when running from CLI using karma-qunit.
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 
 		$query = [
 			'lang' => 'qqx',

includes/specials/SpecialLinkSearch.php

@@ -69,7 +69,7 @@ class SpecialLinkSearch extends QueryPage {
 		$this->outputHeader();
 
 		$out = $this->getOutput();
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 
 		$request = $this->getRequest();
 		$target = $request->getVal( 'target', $par );

includes/specials/SpecialSearch.php

@@ -632,7 +632,7 @@ class SpecialSearch extends SpecialPage {
 		$this->outputHeader();
 		// TODO: Is this true? The namespace remember uses a user token
 		// on save.
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 		$this->addHelpLink( 'Help:Searching' );
 
 		if ( strval( $term ) !== '' ) {

includes/specials/SpecialSpecialpages.php

@@ -36,7 +36,7 @@ class SpecialSpecialpages extends UnlistedSpecialPage {
 		$out = $this->getOutput();
 		$this->setHeaders();
 		$this->outputHeader();
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 		$out->addModuleStyles( 'mediawiki.special' );
 
 		$groups = $this->getPageGroups();

includes/specials/SpecialTrackingCategories.php

@@ -50,7 +50,7 @@ class SpecialTrackingCategories extends SpecialPage {
 		$this->setHeaders();
 		$this->outputHeader();
 		$this->addHelpLink( 'Help:Categories' );
-		$this->getOutput()->allowClickjacking();
+		$this->getOutput()->setPreventClickjacking( false );
 		$this->getOutput()->addModuleStyles( [
 			'jquery.tablesorter.styles',
 			'mediawiki.pager.tablePager'

includes/specials/SpecialVersion.php

@@ -85,7 +85,7 @@ class SpecialVersion extends SpecialPage {
 		$this->setHeaders();
 		$this->outputHeader();
 		$out = $this->getOutput();
-		$out->allowClickjacking();
+		$out->setPreventClickjacking( false );
 
 		// Explode the sub page information into useful bits
 		$parts = explode( '/', (string)$par );

includes/specials/pagers/ContribsPager.php

@@ -706,7 +706,7 @@ class ContribsPager extends RangeChronologicalPager {
 					$this->getAuthority()->probablyCan( 'rollback', $page ) &&
 					$this->getAuthority()->probablyCan( 'edit', $page )
 				) {
-					$this->preventClickjacking();
+					$this->setPreventClickjacking( true );
 					$topmarktext .= ' ' . Linker::generateRollback(
 						$revRecord,
 						$this->getContext(),
@@ -878,8 +878,19 @@ class ContribsPager extends RangeChronologicalPager {
 		}
 	}
 
+	/**
+	 * @deprecated since 1.38, use ::setPreventClickjacking() instead
+	 */
 	protected function preventClickjacking() {
-		$this->preventClickjacking = true;
+		$this->setPreventClickjacking( true );
+	}
+
+	/**
+	 * @param bool $enable
+	 * @since 1.38
+	 */
+	protected function setPreventClickjacking( bool $enable ) {
+		$this->preventClickjacking = $enable;
 	}
 
 	/**

tests/phpunit/includes/OutputPageTest.php

@@ -2442,8 +2442,7 @@ class OutputPageTest extends MediaWikiIntegrationTestCase {
 	}
 
 	/**
-	 * @covers OutputPage::preventClickjacking
-	 * @covers OutputPage::allowClickjacking
+	 * @covers OutputPage::setPreventClickjacking
 	 * @covers OutputPage::getPreventClickjacking
 	 * @covers OutputPage::addParserOutputMetadata
 	 * @covers OutputPage::addParserOutput
@@ -2452,26 +2451,26 @@ class OutputPageTest extends MediaWikiIntegrationTestCase {
 		$op = $this->newInstance();
 		$this->assertTrue( $op->getPreventClickjacking() );
 
-		$op->allowClickjacking();
+		$op->setPreventClickjacking( false );
 		$this->assertFalse( $op->getPreventClickjacking() );
 
-		$op->preventClickjacking();
+		$op->setPreventClickjacking( true );
 		$this->assertTrue( $op->getPreventClickjacking() );
 
-		$op->preventClickjacking( false );
+		$op->setPreventClickjacking( false );
 		$this->assertFalse( $op->getPreventClickjacking() );
 
-		$pOut1 = $this->createParserOutputStub( 'preventClickjacking', true );
+		$pOut1 = $this->createParserOutputStub( 'getPreventClickjacking', true );
 		$op->addParserOutputMetadata( $pOut1 );
 		$this->assertTrue( $op->getPreventClickjacking() );
 
 		// The ParserOutput can't allow, only prevent
-		$pOut2 = $this->createParserOutputStub( 'preventClickjacking', false );
+		$pOut2 = $this->createParserOutputStub( 'getPreventClickjacking', false );
 		$op->addParserOutputMetadata( $pOut2 );
 		$this->assertTrue( $op->getPreventClickjacking() );
 
 		// Reset to test with addParserOutput()
-		$op->allowClickjacking();
+		$op->setPreventClickjacking( false );
 		$this->assertFalse( $op->getPreventClickjacking() );
 
 		$op->addParserOutput( $pOut1 );
@@ -2484,7 +2483,7 @@ class OutputPageTest extends MediaWikiIntegrationTestCase {
 	/**
 	 * @dataProvider provideGetFrameOptions
 	 * @covers OutputPage::getFrameOptions
-	 * @covers OutputPage::preventClickjacking
+	 * @covers OutputPage::setPreventClickjacking
 	 */
 	public function testGetFrameOptions(
 		$breakFrames, $preventClickjacking, $editPageFrameOptions, $expected
@@ -2493,7 +2492,7 @@ class OutputPageTest extends MediaWikiIntegrationTestCase {
 			'BreakFrames' => $breakFrames,
 			'EditPageFrameOptions' => $editPageFrameOptions,
 		] );
-		$op->preventClickjacking( $preventClickjacking );
+		$op->setPreventClickjacking( $preventClickjacking );
 
 		$this->assertSame( $expected, $op->getFrameOptions() );
 	}

tests/phpunit/includes/parser/ParserOutputTest.php

@@ -566,7 +566,7 @@ EOF
 		$b = new ParserOutput();
 		$b->setNoGallery( true );
 		$b->setEnableOOUI( true );
-		$b->preventClickjacking( true );
+		$b->setPreventClickjacking( true );
 		$a->addWrapperDivClass( 'bar' );
 
 		$b->setIndicator( 'zoo', 'Zoo!' );
@@ -580,7 +580,7 @@ EOF
 			'getHideNewSection' => true,
 			'getNoGallery' => true,
 			'getEnableOOUI' => true,
-			'preventClickjacking' => true,
+			'getPreventClickjacking' => true,
 			'getIndicators' => [
 				'foo' => 'Foo!',
 				'bar' => 'Barrr!',