diff --git a/includes/EmailFormatter.php b/includes/EmailFormatter.php index 45b6a4320..d8472b886 100644 --- a/includes/EmailFormatter.php +++ b/includes/EmailFormatter.php @@ -169,28 +169,9 @@ abstract class EchoEmailMode { * @return string */ public static function getNotifIcon( $icon ) { - global $wgEchoNotificationIcons, $wgExtensionAssetsPath, $wgLang; + global $wgLang; - $iconInfo = $wgEchoNotificationIcons[$icon]; - if ( isset( $iconInfo['url'] ) && $iconInfo['url'] ) { - $iconUrl = $iconInfo['url']; - } else { - if ( !isset( $iconInfo['path'] ) || !$iconInfo['path'] ) { - $iconInfo = $wgEchoNotificationIcons['placeholder']; - } - if ( is_array( $iconInfo['path'] ) ) { - $dir = $wgLang->getDir(); - if ( isset( $iconInfo['path'][$dir] ) ) { - $path = $iconInfo['path'][$dir]; - } else { - wfDebugLog( 'Echo', "The \"$icon\" icon does not have anything set for $dir direction." ); - $path = $wgEchoNotificationIcons['placeholder']['path']; // Fallback - } - } else { - $path = $iconInfo['path']; - } - $iconUrl = "$wgExtensionAssetsPath/$path"; - } + $iconUrl = EchoNotificationFormatter::getIconUrl( $icon, $wgLang->getDir() ); return wfExpandUrl( $iconUrl, PROTO_CANONICAL ); } diff --git a/includes/formatters/BasicFormatter.php b/includes/formatters/BasicFormatter.php index d973acbaa..9b2755b4b 100644 --- a/includes/formatters/BasicFormatter.php +++ b/includes/formatters/BasicFormatter.php @@ -181,27 +181,7 @@ class EchoBasicFormatter extends EchoNotificationFormatter { return $this->formatNotificationTitle( $event, $user )->text(); } - $iconInfo = $wgEchoNotificationIcons[$this->icon]; - if ( isset( $iconInfo['url'] ) && $iconInfo['url'] ) { - $iconUrl = $iconInfo['url']; - } else { - if ( !isset( $iconInfo['path'] ) || !$iconInfo['path'] ) { - // Fallback in case icon is not configured; mainly intended for 'site' - $iconInfo = $wgEchoNotificationIcons['placeholder']; - } - if ( is_array( $iconInfo['path'] ) ) { - $dir = $wgLang->getDir(); - if ( isset( $iconInfo['path'][$dir] ) ) { - $path = $iconInfo['path'][$dir]; - } else { - wfDebugLog( 'Echo', "The \"{$this->icon}\" icon does not have anything set for $dir direction." ); - $path = $wgEchoNotificationIcons['placeholder']['path']; // Fallback - } - } else { - $path = $iconInfo['path']; - } - $iconUrl = "$wgExtensionAssetsPath/$path"; - } + $iconUrl = $this->getIconUrl( $this->icon, $wgLang->getDir() ); // Assume html as the format for the notification $output = Html::element( diff --git a/includes/formatters/NotificationFormatter.php b/includes/formatters/NotificationFormatter.php index 3bd0d89b5..0230b383e 100644 --- a/includes/formatters/NotificationFormatter.php +++ b/includes/formatters/NotificationFormatter.php @@ -119,6 +119,51 @@ abstract class EchoNotificationFormatter { return $ts; } + /** + * @todo this shouldn't be static + * @param string $icon Name of icon as registered in BeforeCreateEchoEvent hook + * @param string $dir either 'ltr' or 'rtl' + * @return string + */ + public static function getIconUrl( $icon, $dir ) { + global $wgEchoNotificationIcons, $wgExtensionAssetsPath; + if ( !isset( $wgEchoNotificationIcons[$icon] ) ) { + throw new InvalidArgumentException( "The $icon icon is not registered" ); + } + + $iconInfo = $wgEchoNotificationIcons[$icon]; + $needsPrefixing = true; + + // Now we need to check it has a valid url/path + if ( isset( $iconInfo['url'] ) && $iconInfo['url'] ) { + $iconUrl = $iconInfo['url']; + $needsPrefixing = false; + } elseif ( isset( $iconInfo['path'] ) && $iconInfo['path'] ) { + $iconUrl = $iconInfo['path']; + } else { + // Fallback to hardcoded 'placeholder'. This is used if someone + // doesn't configure the 'site' icon for example. + $icon = 'placeholder'; + $iconUrl = $wgEchoNotificationIcons['placeholder']['path']; + } + + // Might be an array with different icons for ltr/rtl + if ( is_array( $iconUrl ) ) { + if ( !isset( $iconUrl[$dir] ) ) { + throw new UnexpectedValueException( "Icon type $icon doesn't have an icon for $dir directionality" ); + } + + $iconUrl = $iconUrl[$dir]; + } + + // And if it was a 'path', stick the assets path in front + if ( $needsPrefixing ) { + $iconUrl = "$wgExtensionAssetsPath/$iconUrl"; + } + + return $iconUrl; + } + /** * Returns a revision snippet * @param EchoEvent $event The event that the notification is for. diff --git a/tests/phpunit/formatters/NotificationFormatterTest.php b/tests/phpunit/formatters/NotificationFormatterTest.php index dd0024148..26d86727a 100644 --- a/tests/phpunit/formatters/NotificationFormatterTest.php +++ b/tests/phpunit/formatters/NotificationFormatterTest.php @@ -247,4 +247,67 @@ class EchoNotificationFormatterTest extends MediaWikiTestCase { } return $event; } + + /** + * @dataProvider provideGetIconUrl + */ + public function testGetIconUrl( $global, $icon, $dir, $expected, $exception = null ) { + $this->setMwGlobals( array( + 'wgEchoNotificationIcons' => $global, + 'wgExtensionAssetsPath' => 'http://example.org' + ) ); + + if ( $exception ) { + $this->setExpectedException( $exception ); + } + + $url = EchoNotificationFormatter::getIconUrl( $icon, $dir ); + $this->assertEquals( $expected, $url ); + } + + public static function provideGetIconUrl() { + $standard = array( + 'foo' => array( + 'path' => 'foo.png', + ), + 'bar' => array( + 'path' => array( + 'ltr' => 'bar.png' + ), + ), + 'baz' => array( + 'path' => array( + 'ltr' => 'baz-ltr.png', + 'rtl' => 'baz-rtl.png', + ), + ), + 'site' => array( + 'url' => false, + ), + 'placeholder' => array( + 'path' => 'placeholder.png', + ), + ); + $tests = array( + // Standard, no ltr/rtl + array( $standard, 'foo', 'ltr', 'http://example.org/foo.png' ), + array( $standard, 'foo', 'rtl', 'http://example.org/foo.png' ), + // Only ltr configured (bad!) + array( $standard, 'bar', 'ltr', 'http://example.org/bar.png' ), + array( $standard, 'bar', 'rtl', '', 'UnexpectedValueException' ), + // Different for ltr/rtl + array( $standard, 'baz', 'ltr', 'http://example.org/baz-ltr.png' ), + array( $standard, 'baz', 'rtl', 'http://example.org/baz-rtl.png' ), + // Not registered + array( $standard, 'invalid', 'ltr', '', 'InvalidArgumentException' ), + // No url set, fallback to placeholder + array( $standard, 'site', 'ltr', 'http://example.org/placeholder.png' ), + ); + + // Set the 'url', and it doesn't fallback anymore + $standard['site']['url'] = 'http://example.com/site.png'; + $tests[] = array( $standard, 'site', 'ltr', 'http://example.com/site.png' ); + + return $tests; + } }