Once language hierarchy is set up, when the title module (which makes node titles translatable) is also in use, node titles can go missing at /admin/content . This only affects any content that only has translations outside of the current request's fallback chain. Patch to follow...

Comments

james.williams created an issue. See original summary.

james.williams’s picture

Patch followed :-) Hopefully there are enough comments in this to make it clear what is going on! From the Doxygen:

Language hierarchy enforces that languages only fallback to their parent
* languages, whereas Drupal core would allow all languages to be fallbacks.
* Title replacement fields are a special case, since we may want to show a
* title (e.g. to admins) even if there are no translations in a parent language
* so all languages are allowed as fallbacks for those. We give primacy to the
* entity's own language first, then parent languages ahead of cousin languages.
* Since this may mean content is exposed that would not normally be accessible
* to users, a permission is used to toggle this functionality.

james.williams’s picture

Assigned: james.williams » Unassigned
Status: Active » Needs review
steven jones’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

We really need some tests to cover that complicated code :)

steven jones’s picture

Issue tags: +ComputerMinds

Adding a tag so someone at ComputerMinds can pick this up.

steven jones’s picture

Assigned: Unassigned » steven jones

Going to look into writing a test for this one.

james.williams’s picture

Two points of note, from a real-world discussion:

* entity_language() is not used for the final fallback case, which means different behaviour when there is a form language (with entity translation). Is that correct?
* It's actually possible for that final fallback, entity_language(), to return a language that there are not values for, since that might just return the site default language. So we should probably do a final fallback that loops through any remaining languages that haven't been checked, in weight order, until a value is actually found.
* It might even be worth doing an early quick edge case test, if there are no values in any languages, or just one, before the main bulk of the code tries to proceed.

james.williams’s picture

* entity_language() is not used for the final fallback case, which means different behaviour when there is a form language (with entity translation). Is that correct?

This won't matter either way, as form language won't interfere at the point when it gets used. I could believe there is some obscure case when it does matter, but I think let's go with the simpler

* It's actually possible for that final fallback, entity_language(), to return a language that there are not values for, since that might just return the site default language. So we should probably do a final fallback that loops through any remaining languages that haven't been checked, in weight order, until a value is actually found.

My mistake, the patch does do this already.

* It might even be worth doing an early quick edge case test, if there are no values in any languages, or just one, before the main bulk of the code tries to proceed.

This point would still be valid.

steven jones’s picture

Assigned: steven jones » Unassigned
Status: Needs work » Needs review
StatusFileSize
new6.54 KB
new13.83 KB

Right, here's a patch that adds some tests. They need work, and review and probably re-working a little bit. But marking as needs review so that the testbot can have a play.

The gist is that we setup a field that will replace the entity label, and then view the node in various languages and assert that the field value for just that field falls back correctly, as we want to me much more permissive with that one than the others.

I've not implemented @james.williams suggestions from #8 and we should.

steven jones’s picture

Improved patch and added the quick check the alter for a single language to 'fallback' to.

steven jones’s picture

Here is the latest work in progress.

Status: Needs review » Needs work

The last submitted patch, 12: language-hierarchy-empty_node_titles-2596321-12.patch, failed testing.

steven jones’s picture

Status: Needs work » Needs review
StatusFileSize
new26.1 KB

Better tests, that work, and actually test things.

Reckon this is good for commit now.

Status: Needs review » Needs work

The last submitted patch, 14: language-hierarchy-empty_node_titles-2596321-14.patch, failed testing.

steven jones’s picture

Yeah, so the testbot totally didn't download the title module :(

james.williams’s picture

Status: Needs work » Reviewed & tested by the community

New test dependencies don't work nicely with testbot, so let's commit the patch and then run the test again just to confirm it really was that. Looks fine to me otherwise, thanks Steven.

The last submitted patch, 9: language-hierarchy-empty_node_titles-2596321-9.patch, failed testing.

The last submitted patch, 11: language-hierarchy-empty_node_titles-2596321-11.patch, failed testing.

The last submitted patch, 12: language-hierarchy-empty_node_titles-2596321-12.patch, failed testing.

steven jones’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests

Hopefully this is fixed and the branch tests will pass.

Status: Fixed » Needs work

The last submitted patch, 14: language-hierarchy-empty_node_titles-2596321-14.patch, failed testing.

steven jones’s picture

Status: Needs work » Fixed

Stop it testbot!

steven jones’s picture

For clarity, the testbot now says that this branch passes.

Status: Fixed » Closed (fixed)

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