Postponed on #2420737: Differences in dynamic language names are confusing in views, content, etc..

Here's the definition of LANGUAGE_TYPE_URL in locale_language_types_info() (see: http://api.drupal.org/api/drupal/modules!locale!locale.module/function/locale_language_types_info/7)

    LANGUAGE_TYPE_URL => array(
      'fixed' => array(LOCALE_LANGUAGE_NEGOTIATION_URL, LOCALE_LANGUAGE_NEGOTIATION_URL_FALLBACK),
    ),

Is there a particular reason why it doesn't have a name and description? This is a problem whenever someone wants to make this language type configurable.

CommentFileSizeAuthor
#18 d7_views_language_after_patch.jpg46.74 KBpoker10
#18 d7_views_language.jpg96.4 KBpoker10
#18 d9_views_language.jpg115.76 KBpoker10
#18 after_patch_cconfigurable.jpg269.67 KBpoker10
#18 before_patch_configurable.jpg307.08 KBpoker10
#11 language-type-url-1994292-11-D7.patch658 bytesDavid_Rothstein
#4 Monosnap 2015-03-02 12-04-24.png135.27 KBgábor hojtsy
#1 language-type-url-1994292-2-D8.patch689 bytesDavid_Rothstein
#1 language-type-url-1994292-2-D7-do-not-test.patch658 bytesDavid_Rothstein

Comments

a.ross’s picture

a.ross commented
Issue summary: View changes

Add anchor

a.ross’s picture

a.ross commented
Issue summary: View changes

*sigh*

David_Rothstein’s picture

David_Rothstein commented
Title: LANGUAGE_TYPE_URL has no name and description. » LanguageInterface::TYPE_URL (D8) and LANGUAGE_TYPE_URL (D7) have no name or description
Version: 7.x-dev » 8.0.x-dev
Issue summary: View changes
Status: Active » Needs review
Issue tags: +Needs backport to D7
StatusFileSize
new language-type-url-1994292-2-D7-do-not-test.patch658 bytes
new language-type-url-1994292-2-D8.patch689 bytes

Yes, this leads to PHP notices/etc in that case. Since 'name' and 'description' are required, core should be providing them even if it does not make this language type configurable by default.

Here are patches for Drupal 7 and 8.

David_Rothstein’s picture

David_Rothstein commented

Since 'name' and 'description' are required

Actually the hook documentation isn't quite clear about whether they're required or not, but either way it seems like they should be provided.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC, makes sense.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
Issue tags: +D8MI, +language-base, +sprint
StatusFileSize
new Monosnap 2015-03-02 12-04-24.png135.27 KB

This will make the URL method available in views as well, probably confusing users (because you cannot configure this one and URL language does not appear anywhere else but in views then). See #2420737: Differences in dynamic language names are confusing in views, content, etc. for the issue discussing whether to display non-configurable language types in views and the reasons we do so now and distinguish between types with and without names instead. I think #2420737: Differences in dynamic language names are confusing in views, content, etc. would ideally be resolved sooner or elevated in priority if this one is committed.

Screenshot with this patch:

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Status: Needs review » Postponed

I think we have a workable idea at #2420737: Differences in dynamic language names are confusing in views, content, etc., can you help implement?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Postponed » Needs review

#2420737: Differences in dynamic language names are confusing in views, content, etc. finally landed. This should be good to go again.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

I manually tested and this still applied and did not affect the views language list negatively anymore (thanks to #2420737: Differences in dynamic language names are confusing in views, content, etc.), so should be good to go. Yay!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.0.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed c13aadd and pushed to 8.0.x. Thanks!

  • alexpott committed c13aadd on 8.0.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Superb :) Thanks all!

David_Rothstein’s picture

David_Rothstein commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new language-type-url-1994292-11-D7.patch658 bytes

Reuploading the Drupal 7 patch from #2, with the do-not-text prefix removed.

Not sure if there is a similar issue with Views in Drupal 7 as there was in Drupal 8....

fabianx’s picture

fabianx commented
Issue tags: +Needs manual testing

Except for needing to check that, this is RTBC.

  • alexpott committed c13aadd on 8.1.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...

  • alexpott committed c13aadd on 8.3.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...

  • alexpott committed c13aadd on 8.3.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...

  • alexpott committed c13aadd on 8.4.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...

  • alexpott committed c13aadd on 8.4.x 
    Issue #1994292 by David_Rothstein, Gábor Hojtsy: LanguageInterface::...
poker10’s picture

poker10 commented
Issue tags: -Needs manual testing
StatusFileSize
new before_patch_configurable.jpg307.08 KB
new after_patch_cconfigurable.jpg269.67 KB
new d9_views_language.jpg115.76 KB
new d7_views_language.jpg96.4 KB
new d7_views_language_after_patch.jpg46.74 KB

Patch #11 still applies to D7. I have tested it manually (with LANGUAGE_TYPE_URL set as configurable) and it seems working - see screenshots.

before_patch_configurable.jpg - Before patch - PHP warnings and missing name and description
after_patch_cconfigurable.jpg - After patch - no warnings and table data are displayed correctly

To the views - it seems like that in D7 the language configuration is different. There are three general options before and after the patch is applied (current user's language, default site language and language neutral). So I do not see any negative impacts of this patch here either.

d9_views_language.jpg - Views language settings in D9
d7_views_language.jpg - Views language settings in D7
d7_views_language_after_patch.jpg - Views language settings in D7 after applying the patch

So +1 to RTBC.

mcdruid’s picture

mcdruid commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +RTBM

Thanks for doing the follow-up testing with views in D7.

This LGTM!

  • poker10 committed ee6e664 on 7.x 
    Issue #1994292 by David_Rothstein: LanguageInterface::TYPE_URL (D8) and...
poker10’s picture

poker10 commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs backport to D7, -RTBM

Thanks! Commited and pushed to 7.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.