use meta data before searching page by controller

[mutec]

self::getActiveRequest()->getPageID() will check the meta data for a page id before it searches the first match of the controller's classname.
This is also used by the BoxHandler, so it seems to be inconsistent in it's handling. In some special cases (in my case it's a Fireball-Page) the BoyHandler will return a valid page id and assigns the boxes correctly while this method returns null or the first match which is wrong in the most cases.
This change would improve the page-mapping and should not have any unwanted side-effects.

In a nutshell: You can shorten the whole method to

if (self::getActiveRequest() === null) {
	return null;
}

$pageID = self::getActiveRequest()->getPageID();
return $pageID ? PageCache::getInstance()->getPage($pageID) : null;

because it covers both look-ups the method contains (before this commit).

[dtdesign]

From what I can tell, b212ba6 should've resolved this issue already?

[mutec]

No, there was a special case (which is important for a plugin by me), which is not covered, because ['cms']['pageID'] is sent, but it's not a CmsPage. In Request::getPageID() there is no check for CmsPage::class which makes it possible to send a special ID even if the controller would resolve to another page (which would be wrong in my case for 99%):

WCF/wcfsetup/install/files/lib/system/request/Request.class.php

Lines 169 to 185 in b212ba6

	public function getPageID() {
	if ($this->pageID === null) {
	if (isset($this->metaData['cms'])) {
	$this->pageID = $this->metaData['cms']['pageID'];
	}
	else {
	$page = PageCache::getInstance()->getPageByController($this->className);
	if ($page !== null) {
	$this->pageID = $page->pageID;
	}
	else {
	$this->pageID = 0;
	}
	}
	}
	return $this->pageID;

If I'm remembering correctly, this method is also called somewhere else for getting the active page.

[dtdesign]

The cms metadata index is intended for use with the built-in CMS only and as far as I can tell everything works as it is. Have I overlooked something here?

[mutec]

The matching of WoltLab-CMS-pages works perfectly, yes. But the reason I'd need this change is another.

At the moment the system returns the very first match of the controller. But because it's a CMS, there are many items with the same controller but different custom controllers (before you ask: the empty rows are bugs). I tried a few things during beta-stage, but in the end I had to use a custom route to assign the correct controller/classname: https://github.com/codeQuake/Fireball/blob/v3.0/files_wcf/lib/system/request/route/FireballRequestRoute.class.php#L98-L105
Because of this manipulation I'm able to use the WSC's comment system, because PageCommentListBoxController uses RequestHandler::getInstance()->getActiveRequest()->getPageID() which prefers the cms-pageID-parameter even if it's not a WoltLab-CMS-page. Also PageLocationManager uses this parameter without requiring the page to be a WoltLab-CMS-page. The "only" problem for me is the usage of data-page-id="{@$__wcf->getActivePage()->pageID}" data-page-identifier="{$__wcf->getActivePage()->identifier}" within header.tpl to set the pageID and pageIdentifier to the body-tag. And setting the active menu item, but I got a workaround for this problem: https://github.com/codeQuake/Fireball/blob/v3.0/files/lib/page/AbstractCMSPage.class.php#L104-L115
Maybe I overlooked something which would be much better, but that's the case where I'd love to be able to manipulate the pageID using a Route and in my opinion it would be more consistent if you compare it to the other classes I mentioned.

[dtdesign]

You were never supposed to abuse the cms index for your own purposes, especially you should not provide fake page ids. That said, there are two options I'm left with:

1. Leave things as they are, allowing you to use some weird work-around to get your things done.
2. Or enforce the check against the wcf\page\CmsPage-class inside the Request-class too, eventually breaking your implementation.
   I'm not too big into option 2, hence I'm going to stick with the status quo.

PS: I never understood why you opted to reinvent the wheel instead of extending the existing CMS, but that's none of my business.