Steps to reproduce:

  1. Enabled Translation
  2. Add French language
  3. Enable the language switcher block
  4. Create an article

The language switcher blocks shows the french links disabled instead of active and pointing to same node with french UI.

CommentFileSizeAuthor
#27 translation-1060246-27.patch4.53 KBplach
#13 translation-1060246-13.patch4.46 KBplach
#13 translation-1060246-13-test.patch2.98 KBplach
#1 translation-1060246-1.patch2.47 KBplach
#1 translation-1060246-1-test.patch1.41 KBplach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Assigned: plach » Unassigned
Status: Active » Needs review
Issue tags: +Quick fix
FileSize
translation-1060246-1-test.patch1.41 KB
translation-1060246-1.patch2.47 KB

The test-only patch is supposed to fail to show the bug.

montesq’s picture

montesq CreditAttribution: montesq commented
Category: bug » support
Status: Needs review » Closed (works as designed)
Issue tags: -Quick fix

-edit-
cross post

montesq’s picture

montesq CreditAttribution: montesq commented
Category: support » bug
Status: Closed (works as designed) » Needs review
Issue tags: +Quick fix

Status: Needs review » Needs work

The last submitted patch, translation-1060246-1-test.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs work » Needs review

Tests pass and capture the bug. This should be ready to go. Someone please confirm.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Priority: Normal » Major

This looks pretty nasty after all.

montesq’s picture

montesq CreditAttribution: montesq commented
Priority: Major » Normal

This behaviour is explicitly introduced by the function "translation_language_switch_links_alter".
Could you have a look at this code and tell what is wrong for you?
I think this is more a feature request than a bug report...

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Priority: Normal » Major

Sorry for not mentioning before, I thought it was obvious. This is an unwanted regression from the D6 behavior, which I probably introduced in #518364: Nodes with one language don't affect the language switcher block. This is the reason for the major priority.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Perhaps the OP is not clear about this: translation alters the language switcher links even for content types not having multilingual support set to "Enabled, with translation". In this case node translations should not be taken into consideration.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

subscribing...

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Looks good.

ogi’s picture

ogi CreditAttribution: ogi commented

subscribe

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Reviewed & tested by the community » Needs review
Issue tags: -Quick fix
FileSize
translation-1060246-13-test.patch2.98 KB
translation-1060246-13.patch4.46 KB

The previous solution was incomplete as it didn't take into account translations created before disabling translation support for a certain content type and, above all, language neutral nodes which should not have language switch links altered.

Tests are updated accordingly.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Related issue: #1078980: Language neutral not available when there are disabled languages.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Related issue: #1078656: Drupal 7, Multilingual OK, but can't change language on all pages..

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Results are not coming back, but patches pass/fail tests as they are supposed to. We need someone confirming the solution is ok.

Status: Needs review » Needs work

The last submitted patch, translation-1060246-13-test.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs work » Needs review
mattcasey’s picture

mattcasey CreditAttribution: mattcasey commented

subscribing

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

I did not actually test the patch but it looks simple and good, and covers the language neutral and the previously translation enabled nodes as well nicely.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

@Gábor Hojtsy:

Thanks!

Can we have someone confirming the patch works? It has been reviewed and bot tested, we are only missing a human test to RTBC it.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

...would testing the steps described in the issue's description suffice?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

That would be incomplete, you should also test the use cases cited in #13 (they are described in the simpletest comments).

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

k... give me some time to setup a vanilla d7 + i18n.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

i18n is not needed, this is a plain core bug :)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Needs work 
+++ b/modules/translation/translation.module
@@ -503,8 +503,22 @@ function translation_path_get_translations($path) {
+    $node = node_load((int) $matches[1]);
+
+    if (empty($node->tnid)) {
...
+      if ($node->language == LANGUAGE_NONE || !translation_supported_type($node->type)) {

node_load() might return FALSE, in which case the empty() will be TRUE, subsequently triggering PHP notices due to trying to access properties of an object that doesn't exist.

Powered by Dreditor.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs work » Needs review
FileSize
translation-1060246-27.patch4.53 KB

Thanks!
(sun just cannot be cheated ;)

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

...ok got me some time to set it up and configure it (I ended up re-opening #729146: Language switcher block (User interface text language detection) doesn't appear btw) + I needed grasp how core translation actually works.

Anyways, from what I saw the patch in #27 fixes things. So does that count for RBTC? ...well that plus Daniel's review ;)

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

I'd say this should be RTBC, but let's wait and see whether sun has any pending concern.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented

cool ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

I guess this is good to go then.

omercioglu’s picture

omercioglu CreditAttribution: omercioglu commented

subscribing

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Bugs are fixed in the development version first, backported then.

The patch still applies and tests pass, so confirming RTBC status.

HnLn’s picture

HnLn CreditAttribution: HnLn commented

sub

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented

Subscribing

JamFar’s picture

JamFar CreditAttribution: JamFar commented

+1. Subscribing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Issue tags: +translatable fields
Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed this patch to 7.x and 8.x. Thanks for the tests -- we like!

Status: Fixed » Closed (fixed)

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