From 9f3d9b5b239b9ebb3ddfec1c9d2d8d2a54be8f2a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Apr 2017 10:52:17 +0200 Subject: [PATCH] Don't force the use of Accept-Language anymore This is not intended anymore, since it falls back to force english when the header is not set. Also 0228bc6e66cbcb2848eacb41f1de6e7f63ebcb65 makes clear that the order should be: 1. User setting 2. Accept language 3. Admin default This is the case since the commit from above, unless via OCS and DAV. Both forced to accept-language falling back to english. By removing the force, it now also matches the w3 priority list: https://www.w3.org/International/questions/qa-lang-priorities Signed-off-by: Joas Schilling --- lib/private/L10N/Factory.php | 18 ------------- lib/public/L10N/IFactory.php | 8 ------ ocs/v1.php | 3 --- remote.php | 3 --- tests/lib/L10N/FactoryTest.php | 48 ++++++++++++++++------------------ 5 files changed, 22 insertions(+), 58 deletions(-) diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php index 8aad395065c..581ca3fb5d1 100644 --- a/lib/private/L10N/Factory.php +++ b/lib/private/L10N/Factory.php @@ -270,24 +270,6 @@ class Factory implements IFactory { throw new LanguageNotFoundException(); } - /** - * @param string|null $app App id or null for core - * @return string - */ - public function setLanguageFromRequest($app = null) { - - try { - $requestLanguage = $this->getLanguageFromRequest($app); - } catch (LanguageNotFoundException $e) { - $requestLanguage = 'en'; - } - - if ($app === null && !$this->requestLanguage) { - $this->requestLanguage = $requestLanguage; - } - return $requestLanguage; - } - /** * Checks if $sub is a subdirectory of $parent * diff --git a/lib/public/L10N/IFactory.php b/lib/public/L10N/IFactory.php index 0d6e583c7e5..b5f93c2389c 100644 --- a/lib/public/L10N/IFactory.php +++ b/lib/public/L10N/IFactory.php @@ -61,14 +61,6 @@ interface IFactory { */ public function languageExists($app, $lang); - /** - * @param string|null $app App id or null for core - * @return string - * @since 9.0.0 - */ - public function setLanguageFromRequest($app = null); - - /** * Creates a function from the plural string * diff --git a/ocs/v1.php b/ocs/v1.php index e92250b7e38..80df64f4ed6 100644 --- a/ocs/v1.php +++ b/ocs/v1.php @@ -54,9 +54,6 @@ try { // load all apps to get all api routes properly setup OC_App::loadApps(); - // force language as given in the http request - \OC::$server->getL10NFactory()->setLanguageFromRequest(); - OC::$server->getRouter()->match('/ocs'.\OC::$server->getRequest()->getRawPathInfo()); return; } catch (ResourceNotFoundException $e) { diff --git a/remote.php b/remote.php index e5bd3bb8fed..8e74967365d 100644 --- a/remote.php +++ b/remote.php @@ -136,9 +136,6 @@ try { throw new RemoteException('Path not found', OC_Response::STATUS_NOT_FOUND); } - // force language as given in the http request - \OC::$server->getL10NFactory()->setLanguageFromRequest(); - $file=ltrim($file, '/'); $parts=explode('/', $file, 2); diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php index 5e422758cb4..77a56b5f945 100644 --- a/tests/lib/L10N/FactoryTest.php +++ b/tests/lib/L10N/FactoryTest.php @@ -9,6 +9,7 @@ namespace Test\L10N; use OC\L10N\Factory; +use OC\L10N\LanguageNotFoundException; use OCP\IConfig; use OCP\IRequest; use OCP\IUser; @@ -339,26 +340,22 @@ class FactoryTest extends TestCase { public function dataSetLanguageFromRequest() { return [ // Language is available - [null, 'de', null, ['de'], 'de', 'de'], - [null, 'de,en', null, ['de'], 'de', 'de'], - [null, 'de-DE,en-US;q=0.8,en;q=0.6', null, ['de'], 'de', 'de'], + [null, 'de', ['de'], 'de'], + [null, 'de,en', ['de'], 'de'], + [null, 'de-DE,en-US;q=0.8,en;q=0.6', ['de'], 'de'], // Language is not available - [null, 'de', null, ['ru'], 'en', 'en'], - [null, 'de,en', null, ['ru', 'en'], 'en', 'en'], - [null, 'de-DE,en-US;q=0.8,en;q=0.6', null, ['ru', 'en'], 'en', 'en'], - // Language is available, but request language is set - [null, 'de', 'ru', ['de'], 'de', 'ru'], - [null, 'de,en', 'ru', ['de'], 'de', 'ru'], - [null, 'de-DE,en-US;q=0.8,en;q=0.6', 'ru', ['de'], 'de', 'ru'], - - // Request lang should not be set for apps: Language is available - ['files_pdfviewer', 'de', null, ['de'], 'de', ''], - ['files_pdfviewer', 'de,en', null, ['de'], 'de', ''], - ['files_pdfviewer', 'de-DE,en-US;q=0.8,en;q=0.6', null, ['de'], 'de', ''], - // Request lang should not be set for apps: Language is not available - ['files_pdfviewer', 'de', null, ['ru'], 'en', ''], - ['files_pdfviewer', 'de,en', null, ['ru', 'en'], 'en', ''], - ['files_pdfviewer', 'de-DE,en-US;q=0.8,en;q=0.6', null, ['ru', 'en'], 'en', ''], + [null, 'de', ['ru'], new LanguageNotFoundException()], + [null, 'de,en', ['ru', 'en'], 'en'], + [null, 'de-DE,en-US;q=0.8,en;q=0.6', ['ru', 'en'], 'en'], + + // Language for app + ['files_pdfviewer', 'de', ['de'], 'de'], + ['files_pdfviewer', 'de,en', ['de'], 'de'], + ['files_pdfviewer', 'de-DE,en-US;q=0.8,en;q=0.6', ['de'], 'de'], + // Language for app is not available + ['files_pdfviewer', 'de', ['ru'], new LanguageNotFoundException()], + ['files_pdfviewer', 'de,en', ['ru', 'en'], 'en'], + ['files_pdfviewer', 'de-DE,en-US;q=0.8,en;q=0.6', ['ru', 'en'], 'en'], ]; } @@ -367,12 +364,10 @@ class FactoryTest extends TestCase { * * @param string|null $app * @param string $header - * @param string|null $requestLanguage * @param string[] $availableLanguages * @param string $expected - * @param string $expectedLang */ - public function testSetLanguageFromRequest($app, $header, $requestLanguage, array $availableLanguages, $expected, $expectedLang) { + public function testGetLanguageFromRequest($app, $header, array $availableLanguages, $expected) { $factory = $this->getFactory(['findAvailableLanguages']); $factory->expects($this->once()) ->method('findAvailableLanguages') @@ -384,11 +379,12 @@ class FactoryTest extends TestCase { ->with('ACCEPT_LANGUAGE') ->willReturn($header); - if ($requestLanguage !== null) { - $this->invokePrivate($factory, 'requestLanguage', [$requestLanguage]); + if ($expected instanceof LanguageNotFoundException) { + $this->setExpectedException(LanguageNotFoundException::class); + self::invokePrivate($factory, 'getLanguageFromRequest', [$app]); + } else { + $this->assertSame($expected, self::invokePrivate($factory, 'getLanguageFromRequest', [$app]), 'Asserting returned language'); } - $this->assertSame($expected, $factory->setLanguageFromRequest($app), 'Asserting returned language'); - $this->assertSame($expectedLang, $this->invokePrivate($factory, 'requestLanguage'), 'Asserting stored language'); } public function dataGetL10nFilesForApp() { -- 2.39.5