taxonomy_get_tree() incorrect depth on multiple parents

We have the same code in Drupal 8, so first fix this in D8 and add tests to ensure that we never repeat this bug.

Tests for Drupal 8 but doesn't seems that this is failing, NR for testbot.

Trying the tests in Drupal 7.

I can't reproduce the error, we need a programatically failing test for this.

@fiftyz Any additional steps to reproduce the error?

Yeah! I have reproduced the error with a failing test :) This fails only with some tree structures.

Attached two files, the first only the tests and should fail, the second the tests and the fix, should pass.

Easier!!! Why not simply do this?

Yes, it's not correct to only clone the terms with multiple parents because if you have an structure like this:

term1 | depth: 0
-- term2 | depth: 1
---- term3 | depth: 2
term4 | depth: 0
-- term5 | depth: 1
---- term2 | depth: 2
------ term3 | depth: 3

term3 needs to be cloned to correct the depth attribute, but term3 only have one parent (term2). So, if we do this:

if (count($parents[$vid][$term->tid]) > 1) {
  $term = clone $term;
}

term3 is not cloned and it's needed as proven the tests.

In fact we do not need to clone all terms. We need to clone the terms with multiple parents (as term2->term1,term5) and the terms with a single parent that have a parent with a multiple parents (term3->term2->term1,term5), etc... So, I think that the easier and faster way is clone all terms with parents (single or multiple) to correct the depth attributes, instead of recursively see all parents in all depth levels for searching a multiple parent.

if (isset($parents[$vid][$term->tid])) {
  $term = clone $term;
}

Only tests patch should fail to prove the bug, complete patch should pass.

We've tested the patch and seems to work correctly.
Thankyou!

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -1058,9 +1058,8 @@ function taxonomy_get_tree($vid, $parent = 0, $max_depth = NULL, $load_entities
+          // Clone the term to remains correct the depth attribute.

This comment is a bit wonky. And I can't comment on the code. Don't know the internals.

-16 days to next Drupal core point release.

Fixed a few spelling and phrasing issues. I don't understand the approach enough to comment on it's correctness.

Changing the status to attract some eyes to the approach.

Works great and well documented, thx!

Good catch, and nice fix. Thanks for the tests too. Committed to 7.x and 8.x. Thanks!

Same fix for Drupal 6.

Technically I can't put the RTBC but it's the same fix as D7 & D8, testbot it's happy, harmless change, so...

The committed patch results in the taxonomy term object no longer having $term->parents returned from taxonomy_get_tree.

Can you move the line $term->parents = $parents[$vid][$term->tid]; before $term = clone $term; without changing the desired behavior? I tried it and the taxonomy tests still pass.

You cannot use clone() in Drupal 6, as it is a PHP 5 construct and D6 still supports PHP 4. Use drupal_clone() instead. See http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_clo...