Page MenuHomePhabricator

Remove $wgTitle fallback from EditPage in MW1.36
Closed, ResolvedPublic

Description

EditPage.php
	public function getContextTitle() {
		if ( is_null( $this->mContextTitle ) ) {
			wfDebugLog(
				'GlobalTitleFail',
				__METHOD__ . ' called by ' . wfGetAllCallers( 5 ) . ' with no title set.'
			);
			global $wgTitle;
			return $wgTitle;
		} else {
			return $this->mContextTitle;
		}
	}
RELEASE-NOTES-1.30
* Creating an EditPage instance without calling EditPage::setContextTitle() should
  be avoided and will be deprecated in a future release.
RELEASE-NOTES-1.32
* (T176526) EditPage::getContextTitle() falling back to $wgTitle when the
  context title is unset is now deprecated; anything creating an EditPage
  instance should set the context title via ::setContextTitle().

rMW5cd20435dc1a: EditPage: Try to avoid using $wgTitle

Once that rolls out, we should check the logs, deprecate and eventually remove the $wgTitle fallback.

Event Timeline

legoktm@mwlog1001:/srv/mw-log/archive$ zgrep "::getContextTitle" GlobalTitleFail.log-20180611.gz 
2018-06-10 07:57:37 [WxzZ8QpAME4AAKaItQ0AAAAO] mw1243 frwikisource 1.32.0-wmf.7 GlobalTitleFail INFO: EditPage::getContextTitle called by EditPage->edit/EditPage->showEditForm/EditPage->showStandardInputs/EditPage->getCancelLink/EditPage->getContextTitle with no title set.  
2018-06-10 11:32:36 [Wx0MVApAICsAAHzyZ@sAAACG] mw1322 enwikisource 1.32.0-wmf.7 GlobalTitleFail INFO: EditPage::getContextTitle called by ProofreadPage\Page\PageEditAction->show/EditPage->edit/EditPage->showEditForm/EditPage->setHeaders/EditPage->getContextTitle with no title set.  
2018-06-10 11:44:06 [Wx0PBgpAIC4AACpNmKwAAACE] mw1325 enwikisource 1.32.0-wmf.7 GlobalTitleFail INFO: EditPage::getContextTitle called by EditPage->edit/EditPage->showEditForm/EditPage->showStandardInputs/EditPage->getCancelLink/EditPage->getContextTitle with no title set.  
2018-06-10 19:02:42 [Wx110QpAAEMAAAGufZIAAAAJ] mw1272 frwikisource 1.32.0-wmf.7 GlobalTitleFail INFO: EditPage::getContextTitle called by EditPage->edit/EditPage->showEditForm/EditPage->showStandardInputs/EditPage->getCancelLink/EditPage->getContextTitle with no title set.

This only happens on wikis with ProofreadPage enabled seemingly.

Change 439988 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] EditPage::getContextTitle(): Deprecate falling back to $wgTitle

https://gerrit.wikimedia.org/r/439988

Change 439988 merged by jenkins-bot:
[mediawiki/core@master] EditPage::getContextTitle(): Deprecate falling back to $wgTitle

https://gerrit.wikimedia.org/r/439988

Legoktm updated the task description. (Show Details)Jun 14 2018, 3:43 PM
Jdforrester-WMF changed the task status from Open to Stalled.Jun 14 2018, 4:59 PM
Jdforrester-WMF removed a project: Patch-For-Review.

Now deprecated; removal is Stalled awaiting MW1.33.

Aklapper renamed this task from Remove $wgTitle fallback from EditPage to Remove $wgTitle fallback from EditPage in MW1.33.Jun 15 2018, 10:35 AM
Krinkle changed the task status from Stalled to Open.Mar 8 2019, 2:07 AM
Krinkle added a subscriber: Krinkle.

0 results for +channel:GlobalTitleFail AND "getContextTitle" over the last 30 days in WMF Logstash.

Change 498640 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] Remove $wgTitle fallback from MW 1.33

https://gerrit.wikimedia.org/r/498640

xSavitar edited projects, added MW-1.34-release; removed MW-1.33-release.
xSavitar added a subscriber: xSavitar.

We missed 1.33, so moved to 1.34 and will fix the patch to conform.

xSavitar renamed this task from Remove $wgTitle fallback from EditPage in MW1.33 to Remove $wgTitle fallback from EditPage in MW1.34.Apr 26 2019, 3:09 PM

Change 498640 abandoned by D3r1ck01:
EditPage: Remove $wgTitle fallback from MW 1.33

Reason:
Made I51e0a4b7f8e2441bdd5f5fe98d20175ea79072dc instead.

https://gerrit.wikimedia.org/r/498640

Change 506695 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] EditPage: Remove $wgTitle global fallback from EditPage

https://gerrit.wikimedia.org/r/506695

xSavitar claimed this task.Apr 26 2019, 3:44 PM
Restricted Application added a project: User-xSavitar. · View Herald TranscriptApr 26 2019, 3:44 PM
xSavitar moved this task from Backlog to Under Review on the User-xSavitar board.Apr 26 2019, 3:45 PM
xSavitar removed xSavitar as the assignee of this task.May 18 2019, 2:56 PM
xSavitar removed a project: User-xSavitar.

Change 506695 abandoned by D3r1ck01:
EditPage: Remove $wgTitle global fallback from EditPage

https://gerrit.wikimedia.org/r/506695

Hey there, should this be moved to 1.35? The cut is a couple of weeks away. If it needs to go out in 1.34, is there anything I can do to help get it out of the door?

Apparently, the main source of warnings is CodeEditorHooks::editPageShowEditFormInitial. I wonder if that method should be changed to just use $wgTitle, since that's what it's already doing under the hood.

Apparently, the main source of warnings is CodeEditorHooks::editPageShowEditFormInitial. I wonder if that method should be changed to just use $wgTitle, since that's what it's already doing under the hood.

Lovely. Yes, just kill it in CodeEditor.

Change 539922 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/CodeEditor@master] Directly use $wgTitle instead of relying on getContextTitle fallback

https://gerrit.wikimedia.org/r/539922

Change 539922 merged by jenkins-bot:
[mediawiki/extensions/CodeEditor@master] Directly use $wgTitle instead of relying on getContextTitle fallback

https://gerrit.wikimedia.org/r/539922

Moving to next milestone. There have been two release candidates for 1.34 already. Seems a bit late to backport removal if and when it gets drafted.

I note that we have many instances of this warning on logstash (500 in the past 24 hours). Unfortunately, wfDeprecated's logging is completely useless. It just says

Called from EditPage::setHeaders

or

Called from EditPage::displayViewSourcePage

but it doesn't say what the actual caller (i.e. outside of EditPage) is.

Change 561386 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] EditPage: Include class name ::getContextTitle() deprecation warning

https://gerrit.wikimedia.org/r/561386

Change 561386 merged by jenkins-bot:
[mediawiki/core@master] EditPage: Include class name ::getContextTitle() deprecation warning

https://gerrit.wikimedia.org/r/561386

Change 561392 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/LiquidThreads@master] Ensure we always call EditPage::setContextTitle() when creating a new EditPage

https://gerrit.wikimedia.org/r/561392

Change 561439 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/JsonConfig@master] Use AlternateEdit hook to override EditPage::$contentFormat

https://gerrit.wikimedia.org/r/561439

I note that we have many instances of this warning on logstash (500 in the past 24 hours). Unfortunately, wfDeprecated's logging is completely useless. It just says

Called from EditPage::setHeaders

or

Called from EditPage::displayViewSourcePage

but it doesn't say what the actual caller (i.e. outside of EditPage) is.

I recommend looking at the URL field too, usually it gives a hint as to what extension or code path is being called to ease searching...which is how I found LQT and JsonConfig.

Based on https://codesearch.wmflabs.org/search/?q=new%20(%5C%5C)%3FEditPage&i=nope&files=&repos= those look like the only remaining Wikimedia-deployed stuff.

I recommend looking at the URL field too, usually it gives a hint as to what extension or code path is being called to ease searching...which is how I found LQT and JsonConfig.

Thanks. I noted that various requests where on the Data: namespace, but I don't know what it's for...

Change 561439 merged by jenkins-bot:
[mediawiki/extensions/JsonConfig@master] Use AlternateEdit hook to override EditPage::$contentFormat

https://gerrit.wikimedia.org/r/561439

Jdforrester-WMF renamed this task from Remove $wgTitle fallback from EditPage in MW1.34 to Remove $wgTitle fallback from EditPage in MW1.35.Jan 6 2020, 6:45 PM

Change 561392 merged by jenkins-bot:
[mediawiki/extensions/LiquidThreads@master] Ensure we always call EditPage::setContextTitle() when creating a new EditPage

https://gerrit.wikimedia.org/r/561392

I think we're set to do this, but this is notoriously hard to debug, so I'd rather push this to 1.36 and merge after the branch cut.

Legoktm renamed this task from Remove $wgTitle fallback from EditPage in MW1.35 to Remove $wgTitle fallback from EditPage in MW1.36.Jul 13 2020, 10:09 PM
Legoktm edited projects, added MW-1.36-release; removed MW-1.35-release.

Change 612428 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Throw exception if EditPage has no context title set

https://gerrit.wikimedia.org/r/612428

Legoktm claimed this task.Jul 15 2020, 4:30 AM

Change 612428 merged by jenkins-bot:
[mediawiki/core@master] Throw exception if EditPage has no context title set

https://gerrit.wikimedia.org/r/612428

Jdforrester-WMF closed this task as Resolved.Sep 15 2020, 3:05 PM