Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkp/pkp-lib#9910 Use UI Language in Web Feeds #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion WebFeedGatewayPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
use Exception;
use PKP\category\Category;
use PKP\core\Registry;
use PKP\facades\Locale;
use PKP\i18n\LocaleConversion;
use PKP\plugins\GatewayPlugin;

class WebFeedGatewayPlugin extends GatewayPlugin
Expand Down Expand Up @@ -126,7 +128,8 @@ public function fetch($args, $request): bool
'latestDate' => $latestDate,
'feedUrl' => $request->getRequestUrl(),
'userGroups' => $userGroups,
'includeIdentifiers' => $includeIdentifiers
'includeIdentifiers' => $includeIdentifiers,
'language' => locale_get_region(Locale::getLocale()) ? locale_get_primary_language(Locale::getLocale()) . '-' . locale_get_region(Locale::getLocale()) : locale_get_primary_language(Locale::getLocale()),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasraoni, what about this solution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PKP Locale class doesn't provide such things, so I agree with the fix. Another option is to make the Locale::_parse() method public.


About the code, I'd just store the locale_get_region(Locale::getLocale()) and locale_get_primary_language(Locale::getLocale()) in variables, to make the code shorter and perhaps use the class-based version of \Locale::getRegion() and \Locale::getPrimaryLanguage().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possible problem, that makes me think that it's better to make that method public is that perhaps some PHP locales are not very compatible with our ones.

I mean, we have be@cyrillic, but a call to Locale::getScript('be@cyrillic') won't bring what we would expect, while be-Cyrl will.

]
)
->setHeaders(['content-type: ' . static::FEED_MIME_TYPE[$feedType] . '; charset=utf-8'])
Expand Down
2 changes: 1 addition & 1 deletion templates/rss.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
{/if}

{if $context->getPrimaryLocale()}
<dc:language>{$context->getPrimaryLocale()|replace:'_':'-'|strip|escape:"html"}</dc:language>
<dc:language>{$language|escape}</dc:language>
{/if}

<prism:publicationName>{$context->getLocalizedName()|strip|escape:"html"}</prism:publicationName>
Expand Down
2 changes: 1 addition & 1 deletion templates/rss2.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

{* optional elements *}
{if $context->getPrimaryLocale()}
<language>{$context->getPrimaryLocale()|replace:'_':'-'|strip|escape:"html"}</language>
<language>{$language|escape}</language>
{/if}

{if $context->getLocalizedData('licenseTerms')}
Expand Down