fix infinite loop and programmatic consumption tracking

Signed-off-by: Tobias Möritz <tobias@moeritz.io>
tobimori · Mar 15, 2024 · 41dc8b2 · 41dc8b2
1 parent 72d95c6
commit 41dc8b2
Showing 1 changed file with 52 additions and 39 deletions.
diff --git a/classes/Meta.php b/classes/Meta.php
@@ -52,7 +52,7 @@ protected function metaArray(): array
 		 */
 		$meta =
 			[
-				'title' => 'title',
+				'title' => 'metaTitle',
 				'description' => 'metaDescription',
 				'date' => fn () => $this->page->modified($this->dateFormat()),
 				'og:title' => 'ogTitle',
@@ -72,31 +72,36 @@ protected function metaArray(): array
 		}
 
 		// only add canonical and alternate tags if the page is indexable
-		if (!$robotsActive || !Str::contains($this->robots(), 'noindex')) {
-			$meta['canonical'] = 'canonicalUrl';
-			$meta['og:url'] = 'canonicalUrl';
-
-			// Multi-lang
-			if (kirby()->languages()->count() > 1 && kirby()->language() !== null) {
-				foreach (kirby()->languages() as $lang) {
-					$meta['alternate'][] = [
-						'hreflang' => fn () => $lang->code(),
-						'href' => fn () => $this->page->url($lang->code()),
-					];
-
-					if ($lang !== kirby()->language()) {
-						$meta['og:locale:alternate'][] = fn () => $lang->code();
-					}
+		// we have to resolve this lazily (using a callable) to avoid an infinite loop
+		$allowsIndexFn = fn () => !$robotsActive || !Str::contains($this->robots(), 'noindex');
+
+		// canonical
+		$canonicalFn = fn () => $allowsIndexFn() ? $this->canonicalUrl() : null;
+		$meta['canonical'] = $canonicalFn;
+		$meta['og:url'] = $canonicalFn;
+
+		// Multi-lang alternate tags
+		if (kirby()->languages()->count() > 1 && kirby()->language() !== null) {
+			foreach (kirby()->languages() as $lang) {
+				// only add alternate tags if the page is indexable
+				$meta['alternate'][] = fn () => $allowsIndexFn() ? [
+					'hreflang' => $lang->code(),
+					'href' => $this->page->url($lang->code()),
+				] : null;
+
+				if ($lang !== kirby()->language()) {
+					$meta['og:locale:alternate'][] = fn () => $lang->code();
 				}
-
-				$meta['alternate'][] = [
-					'hreflang' => fn () => 'x-default',
-					'href' => fn () => $this->page->url(kirby()->language()->code()),
-				];
-				$meta['og:locale'] = fn () => kirby()->language()->locale(LC_ALL);
-			} else {
-				$meta['og:locale'] = fn () => $this->locale(LC_ALL);
 			}
+
+			// only add alternate tags if the page is indexable
+			$meta['alternate'][] = fn () => $allowsIndexFn() ? [
+				'hreflang' => 'x-default',
+				'href' => $this->page->url(kirby()->language()->code()),
+			] : null;
+			$meta['og:locale'] = fn () => kirby()->language()->locale(LC_ALL);
+		} else {
+			$meta['og:locale'] = fn () => $this->locale(LC_ALL);
 		}
 
 		// Twitter tags "opt-in" - TODO: wip
@@ -173,12 +178,9 @@ public function snippetData(array $raw = null): array
 
 			// allow overrides from metaDefaults for keys that are a callable or array by default
 			// (all fields from meta array that are not part of the regular cascade)
-
-			if (is_callable($value) || is_array($value)) {
-				if ($mergeWithDefaults && array_key_exists($name, $this->metaDefaults)) {
-					$this->consumed[] = $name;
-					$value = $this->metaDefaults[$name];
-				}
+			if ((is_callable($value) || is_array($value)) && $mergeWithDefaults && array_key_exists($name, $this->metaDefaults)) {
+				$this->consumed[] = $name;
+				$value = $this->metaDefaults[$name];
 			}
 
 			// if the value is a string, we know it's a field name
@@ -296,8 +298,13 @@ public function get(string $key, array $exclude = []): Field
 		}
 
 		// Track consumed keys, so we don't output legacy field values
-		if (A::has($this->metaDefaults, $key)) {
-			$this->consumed[] = $key;
+		$toBeConsumed = $key;
+		if (
+			(array_key_exists($toBeConsumed, $this->metaDefaults)
+				|| array_key_exists($toBeConsumed = $this->findTagForField($toBeConsumed), $this->metaDefaults))
+			&& !in_array($toBeConsumed, $this->consumed)
+		) {
+			$this->consumed[] = $toBeConsumed;
 		}
 
 		foreach (array_diff($cascade, $exclude) as $method) {
@@ -370,6 +377,12 @@ protected function fallbackFields(string $key): Field|null
 		return null;
 	}
 
+
+	protected function findTagForField(string $fieldName): string|null
+	{
+		return array_search($fieldName, $this->metaArray());
+	}
+
 	/**
 	 * Get the meta value for a given key from the page's meta
 	 * array, which can be set in the page's model metaDefaults method
@@ -389,7 +402,7 @@ protected function programmatic(string $key): Field|null
 		 * try looking it up in the meta array
 		 * maybe it is a meta tag and not a field name?
 		 */
-		if (!isset($val) && ($key = array_search($key, $this->metaArray())) && array_key_exists($key, $this->metaDefaults)) {
+		if (!isset($val) && ($key = $this->findTagForField($key)) && array_key_exists($key, $this->metaDefaults)) {
 			$val = $this->metaDefaults[$key];
 		}
 
@@ -411,7 +424,7 @@ protected function programmatic(string $key): Field|null
 			}
 
 			if (is_a($val, 'Kirby\Content\Field')) {
-				return $val;
+				return new Field($this->page, $key, $val->value());
 			}
 
 			return new Field($this->page, $key, $val);
@@ -491,10 +504,10 @@ private function canInherit(string $key): bool
 	/**
 	 * Applies the title template, and returns the correct title
 	 */
-	public function title()
+	public function metaTitle()
 	{
-		$title = $this->metaTitle();
-		$template = $this->metaTemplate();
+		$title = $this->get('metaTitle');
+		$template = $this->get('metaTemplate');
 
 		$useTemplate = $this->page->useTitleTemplate();
 		$useTemplate = $useTemplate->isEmpty() ? true : $useTemplate->toBool();
@@ -519,8 +532,8 @@ public function title()
 	 */
 	public function ogTitle()
 	{
-		$title = $this->metaTitle();
-		$template = $this->ogTemplate();
+		$title = $this->get('metaTitle');
+		$template = $this->get('ogTemplate');
 
 		$useTemplate = $this->page->useOgTemplate();
 		$useTemplate = $useTemplate->isEmpty() ? true : $useTemplate->toBool();