Page MenuHomePhabricator

Inconsistencies in Title->isValid and Title::newFromText documentation.
Closed, ResolvedPublic

Description

The Title::newFromText is documented as follows:

	 * Title objects returned by this method are guaranteed to be valid, and
	 * thus return true from the isValid() method.

however, this is not true. A simple example would be Title::newFromText( '#' )->isValid() === false.

I've modified one of the Title object tests to show that inconsistency is more widespread than just a single corner case. See https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/527660 and in particular the test failures for TitleTest there.

I'm wondering why Title::newFromText and Title::isValid use quite different (but similar) code paths? How about merging the two, like https://gerrit.wikimedia.org/r/c/mediawiki/core/+/527640 ?

Details

Related Gerrit Patches:

Event Timeline

Pchelolo created this task.Aug 2 2019, 9:52 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 2 2019, 9:52 PM
Pchelolo renamed this task from Inconsistencies in Title->isValid to Inconsistencies in Title->isValid and Title::newFromText documentation..Aug 2 2019, 9:53 PM

Assuming this task is about MediaWiki-General, hence adding project tag so others can find this task when searching for tasks under that code project.

WDoranWMF triaged this task as High priority.Aug 6 2019, 6:25 PM
WDoranWMF added a subscriber: WDoranWMF.

Relying on a guarantee that is not there, will cause ongoing problems if - at least - the documentation error is not fixed.

daniel claimed this task.Aug 21 2019, 12:20 PM

Change 533271 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Title: make newFromText, isValid, and canExist behave consistently.

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

Anomie added a subscriber: Anomie.

Moving this back to "Doing"; patch currently has a -1 from the author.

Change 533271 merged by jenkins-bot:
[mediawiki/core@master] Title: make newFromText, isValid, and canExist behave consistently.

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

Anomie closed this task as Resolved.Oct 8 2019, 2:15 PM