Page MenuHomePhabricator

please correct method "getBaseText()" and "getSubpageText()" to return the actual name of the pages
Open, MediumPublicFeature

Description

these two methods has serious problems, because it returns the correct name of the sub pages and base pages, for exemple

Guia do Linux/Avançado/Personalização do Sistema/Arquivo /etc/profile

the subpage would be correctly defined to "Arquivo /etc/profile" instead of "profile". In this example, the last "/" is part of the chapter name, which is "Arquivo /etc/profile", and not a separator. the same goes for base pages.
Note that by default, MediaWiki shows below the title of a page (in a span inside of the element with id="contentSub") the names of "existing pages above" each page in a namespace with subpages enabled.

for this reason (with the help of my friend Helder.wiki) made ​​a code that corrects this failure and expect that we deserve some attention.
here is the code :
/**

  • Get the name of the lowest-level parent page which exists and returns the real subpage or basepage *
  • @return String Base page name or Sub page name
	 */

	 private function CheckExistence($type = null){
		if ( !MWNamespace::hasSubpages( $this->mNamespace ) ) {
			return $this->getText();
		}
		$defaultpage = $this->mTextform;
		$page = $this->mTextform;
		for( $i = 1; $i <= 25; $i++ ){
			$pos = strrpos( $page, '/' );
			if ( $pos ) {
				$page = substr( $page, 0, $pos );
				if( Title::newFromText( $page )->exists()  ){
					switch( $type ){
					case 'subpage' :				
						return (substr( $defaultpage, $pos+1 ));
					case 'basepage': 
						return ($page);
					default:
						return( $defaultpage); 
					}
				}
			}
			else{
				return( $defaultpage );
			}
		}
		return '';

}

/**

  • Get the base name, i.e. the leftmost parts before the / *
  • @return String Base name
	 */

public function getBaseText() {

		$page = $this->mTextform;
		if($this->CheckExistence('basepage')){
			return $this->CheckExistence('basepage');
		}
		return substr( $page, 0, strrpos( $page, '/' ) );

}

/**

  • Get the lowest-level subpage name, i.e. the rightmost part after / *
  • @return String Subpage name
	 */

public function getSubpageText() {

		$page = $this->mTextform;
		if($this->CheckExistence('subpage')){
			return $this->CheckExistence('subpage');
		}
		return substr( $page, strrpos( $page, '/' )+1 );

}

This code needs to be added "Title.php" for replace the "getSubpageText()" getBaseText() "and added the private method "CheckExistence() "


Version: unspecified
Severity: enhancement

Details

Reference
bz27976

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:28 PM
bzimport set Reference to bz27976.
bzimport added a subscriber: Unknown Object (MLST).
  • in the first line I meant "no returns"

What do you propose to happen in the following cases:

"Foo" does not exist
"Foo/bar" exists ... what is its title?

"Foo" exists
"Foo/bar " exists
"Foo/bar /bash/" does not exist
"Foo/bar /bash/blah" exists. ... what is its title?

"Foo" does not exist
"Foo/bar " does not exist
"Foo/bar /bash/" exists ... what is its title?
"Foo/bar /bash/blah" exists. ... what is its title?

I thank you for your code contribution, but I would think some parser tests here would help, too.

In the first case

If the page is parsed "Foo" the result to be basepage "Foo"  and subpage too
If the page is parsed "Foo/bar" the result to be basepage "Foo/bar" and subpage too(if there is no father then the page means that the "/" part of the name of the page)

In the second case

If the page is parsed "Foo" the result to be basepage "Foo" and subpage too
If the page is parsed "Foo/bar" the result to be basepage "Foo" and subpage "bar"
If the page is parsed "Foo/bar/bash" the result to be basepagee "Foo/bar"  and subpage "bash"
If the page is parsed "Foo/bar/bash/blah" the result to be basepage "Foo/bar" and subpage "bash/blah"

In the third case

If the page is parsed "Foo" the result to be basepage "Foo" and subpage too
If the page is parsed "Foo/bar" the result to be basepage "Foo/bar" and subpage too(if there is no father then the page means that the "/" part of the name of the page) 
If the page is parsed "Foo/bar/bash" the result to be basepage "Foo/bar"  and subpage "Foo/bar/bash"(if there is no father then the page means that the "/" part of the name of the page) 
If the page is parsed "Foo/bar/bash/blah" the result to be basepage "Foo/bar/bash" and subpage "blah"

Created attachment 8280
New code

(In reply to comment #0)
Here is the patch with the code above

Attached:

At least fiwp has pages like Talk:M/S_Jamaa_II where M is not supposed to be the basepagename.

[Mid-air collision detected!]

(In reply to comment #2)
For the sake of comparison, with the old version, if we add the wikicode
*base = "{{BASEPAGENAME}}"
*sub = "{{SUBPAGENAME}}"
to the following pages in the described situations, we have:
(1)

"Foo" does not exist
"Foo/bar" exists ... what is its title?

On "Foo":

*contentSub is empty (since there is no bar, we are not in a subpage)
*base = "Foo"
*sub = "Foo"

On "Foo/bar":

*contentSub is empty (since the existence of a bar doesn't means we are in a

subpage)

*base = "Foo" (wrong)
*sub = "bar" (wrong)

(2)

"Foo" exists
"Foo/bar " exists
"Foo/bar /bash/" does not exist
"Foo/bar /bash/blah" exists. ... what is its title?

On "Foo":

*contentSub is empty (since there is no bar, we are not in a subpage)
*base = "Foo"
*sub = "Foo"

On "Foo/bar": (note that "Foo/bar " can't exist with a space in the end of
title)

*contentSub has "< Foo" (here the bar is a separator, so we are in a subpage)
*base = "Foo"
*sub = "bar"

On "Foo/bar /bash/":

*contentSub has "< Foo | bar" (we are in a subpage of "Foo/bar" whose name is

"bash/")

*base = "Foo/bar /bash" (wrong)
*sub = "" (wrong)

On "Foo/bar /bash/blah":

*contentSub has "< Foo | bar" (we are in another subpage of "Foo/bar" whose

name is "bash/blah")

*base = "Foo/bar /bash" (wrong)
*sub = "blah" (wrong)

(3)

"Foo" does not exist
"Foo/bar " does not exist
"Foo/bar /bash/" exists ... what is its title?
"Foo/bar /bash/blah" exists. ... what is its title?

On "Foo":

*contentSub is empty (since there is no bar, we are not in a subpage)
*base = "Foo"
*sub = "Foo"

On "Foo/bar":(note that "Foo/bar " can't exist with a space in the end of
title)

*contentSub is empty (since the existence of a bar doesn't means we are in a

subpage)

*base = "Foo" (wrong)
*sub = "bar" (wrong)

On "Foo/bar /bash/":

*contentSub is empty (none of the bars is used to separate "the title of an

existing page" of the rest of the title, so we are not in a subpage)

*base = "Foo/bar /bash" (wrong)
*sub = "" (wrong)

On "Foo/bar /bash/blah":

*contentSub is empty (same as before)
*base = "Foo/bar /bash" (wrong)
*sub = "blah" (wrong)

On "Foo/bar /bash//blah"

*contentSub is "< Foo/bar /bash/"
*base = "Foo/bar /bash/"
*sub = "blah"

Niklas Laxström, Really... but that's the thing that happens in both the
{{BASEPAGENAME}} today, as our new code, our intention with this new code is to
solve a different type of inconsistency that has no direct relationship with
it.

Parser test with many possibilities

Attached:

Comment on attachment 8280
New code

(In reply to comment #2)
Here is the parserTests above

Comment on attachment 8558
Parser test with many possibilities

(In reply to comment #2)
Here is the parserTests above(please ignore my last comment)

My main concerns with this code are as follows:

*This does existence checks (and possibly quite a few [like ~127] for weird inputs) whenever getSubpage is called. I don't know for sure, but that sounds expensive when (an abusive) user could add thousands of such things to a page (I suppose if we really wanted this we could make it an expensive parser function)
*People may be (ab)using the {{SUPAGENAME}} stuff for other uses that this would break (admittedly I don't know off hand if anyone does, but we should be careful when changing these types of thing). For example {{BASEPAGE:foo/bar}} can be used to strip off everything after the / without any concern for if its a real title. (Perhaps if we really wanted it, make a new magic parser func thing for this behaviour)

Minor issue: (This is just at looking at code, I haven't tested so i could of missed something). It appears this doesn't check existence in the right namespace.

sumanah wrote:

Adding the "reviewed" keyword since Bawolff has reviewed it in comment # 11. Raylton, have you had a chance to consider those comments? Thanks for the patch and the parser test!

Change 154377 had a related patch set uploaded by Helder.wiki:
Allow subpagename to contain bars

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

Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 12:24 PM
Aklapper removed a subscriber: Kosikfl.