Problem/Motivation

Taxonomy terms that have more children than will fit on one page (>100) then when paging to next page term page becomes blank.

  1. Create a new vocabulary (/admin/content/taxonomy/add/vocabulary)
  2. Run drush ev '$config = \Drupal::configFactory()->getEditable("taxonomy.settings");$config->set('terms_per_page_admin', 3);$config->save(); or drush vset taxonomy_terms_per_page_admin 3' in D7, or modify drupal8\core\modules\taxonomy\config\install\taxonomy.settings.yml file manually, and change terms_per_page_admin to the value of 3.
  3. Add 5 terms than will fit onto the "list terms" page.
  4. Make sure that one parent has more than 100 children No need for this, if you modify the terms_per_page_admin value. Instead use the number you have set + 2.
  5. On page one everything is working (screenshot 1)
  6. Click on page 2 (or "next") on the "list terms" page
  7. It will go to the the next page but says "No terms available."(screenshot 2)

The whole behavior and codebase is confusing.

Remaining tasks

**Fix page 2**
Patch in #33 gets terms to show on page two but they are the same terms from page 1.

**Write Tests**
Test needs to be written to verify.
-Generate 150 terms
-Set all terms to be a child of one term
-View admin/structure/taxonomy/[vocab]
-Verify terms are showing as children of parent on page 1
-Navigate to page 2 and ensure parent term shows again and other terms.
-Also ensure on page two that terms are different than page 1.

Original report by whatistocome

To re-produce issue:
* Create a new vocabulary (/admin/content/taxonomy/add/vocabulary)
* Add more terms than will fit onto the "list terms" page
* Click on page 2 (or "next") on the "list terms" page
* It will go to the the next page but says "No terms available."

CommentFileSizeAuthor
#98 242324-98-do-not-test.patch8.44 KBdouggreen
#97 242324-97-do-not-test.patch8.47 KBchx
#87 242324_87.patch13.12 KBbalagan
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 242324_87.patch. Unable to apply patch. See the log in the details link for more information. View
#78 242324_78.patch13.13 KBchx
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 242324_78.patch. Unable to apply patch. See the log in the details link for more information. View
#77 242324_75-taxonomy-admin-tree-broken.patch5.08 KBchx
PASSED: [[SimpleTest]]: [MySQL] 41,106 pass(es). View
#75 242324_75-taxonomy-admin-tree-broken.patch5.11 KBchx
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 242324_75-taxonomy-admin-tree-broken.patch. Unable to apply patch. See the log in the details link for more information. View
#70 interdiff.txt1.21 KBchx
#70 242324_70.patch5.85 KBchx
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 242324_70.patch. Unable to apply patch. See the log in the details link for more information. View
#69 interdiff.242324.66.69.txt2.82 KBrychannel
#69 242324.69.patch5.29 KBrychannel
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,372 pass(es). View
#68 242324.68.patch5.29 KBrychannel
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,386 pass(es). View
#68 interdiff.242324.66.68.txt2.82 KBrychannel
#66 242324.66.patch5.34 KBrychannel
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,180 pass(es). View
#62 242324.62.patch3.53 KBrychannel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,334 pass(es), 19 fail(s), and 0 exception(s). View
#62 interdiff.242324.59.62.txt1.94 KBrychannel
#59 242324.59.patch4.58 KBrychannel
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,354 pass(es). View
#57 interdiff.242324.55.57.txt3.18 KBrychannel
#57 242324.57.tests-only.patch2.77 KBrychannel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,533 pass(es), 19 fail(s), and 0 exception(s). View
#55 interfdiff.242324.52.55.txt663 bytesrychannel
#55 242324.55.tests-only.patch2.68 KBrychannel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,369 pass(es), 41 fail(s), and 777 exception(s). View
#52 interdiff.242324.50.52.txt390 bytesrychannel
#52 242324.52.tests-only.patch2.67 KBrychannel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,068 pass(es), 1 fail(s), and 0 exception(s). View
#50 242324.50.tests-only.patch2.67 KBrychannel
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,913 pass(es), 1 fail(s), and 0 exception(s). View
#42 242324-42.patch4.34 KBcgalli
FAILED: [[SimpleTest]]: [MySQL] 55,472 pass(es), 174 fail(s), and 570 exception(s). View
#41 242324-41.patch4.34 KBLinL
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 242324-41.patch. Unable to apply patch. See the log in the details link for more information. View
#40 242324-40-FAIL.patch2.72 KBkbasarab
FAILED: [[SimpleTest]]: [MySQL] 41,433 pass(es), 19 fail(s), and 0 exception(s). View
#40 242324-40.patch4.32 KBkbasarab
PASSED: [[SimpleTest]]: [MySQL] 41,452 pass(es). View
#39 page1.png41.34 KBkbasarab
#39 page2.png46.7 KBkbasarab
#39 page3.png31.04 KBkbasarab
#39 page4.png24.07 KBkbasarab
#36 page1.png48.73 KBCOBadger
#36 page2.png50.35 KBCOBadger
#33 242324-33.patch1.61 KBkbasarab
PASSED: [[SimpleTest]]: [MySQL] 40,391 pass(es). View
#32 screenshot 1.jpg255.1 KBTimVS
#32 screenshot 2.jpg148.89 KBTimVS
#23 242324-23-taxonomy-empty_page.patch1.54 KBpfrenssen
PASSED: [[SimpleTest]]: [MySQL] 35,743 pass(es). View
#15 taxonomy-242324-20080422-1.patch1.84 KBwonder95
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-242324-20080422-1_0.patch. View
#2 taxonomy-242324-20080422-1.patch1.84 KBtriclops
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-242324-20080422-1.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

triclops’s picture

Version: 6.1 » 6.2

I am encountering a similar issue.

We have a taxonomy with around 20-30 root terms and a few hundred terms sitting below each of those. The first page of admin/content/taxonomy/1 shows a total of 53 terms ($page_increment is 10) and only 2 of those terms are 'root' terms. The first has 38 descendants and the second 13.

Clicking 'next' will got to the next page but it says 'No terms available'. In fact pages 2, 3, and 4 all show 'No terms available'.

Page 5 will then show the 2nd root term shown on Page 1, it's 13 descendants and the next two root terms (which have no descendants). From page 5 onward the term listing appears to work more or less correctly.

triclops’s picture

Status: Active » Needs review
FileSize
1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-242324-20080422-1.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View

The bug is in taxonomy_overview_terms() in the do {} while() loop that builds the list of terms for the page and only occurs if the 1st root term in a vocabulary has more than a page of descendants (i.e. 10 or more).

This is because it is trying to rewind the internal array pointer in $tree using prev() to before the beginning of the array so the second page starts with a root term. Doing so causes subsequent calls to next() (or current() or prev()) to return FALSE, aborting the loop early.

I have attached a patch to fix this bug. Instead of using prev() and next() it uses an index instead - which is allowed to go negative.

agentrickard’s picture

Patch works as designed. This seems to be a good fix to the problem described.

Crell’s picture

I can confirm this bug as well.

Coding style looks overall good, but I don't get $back_peddle. Seems like an odd variable name. It should probably either have a more descriptive name or better inline docs to explain what it's doing.

coupet’s picture

I can confirm bug! subscribing.

earnie’s picture

Status: Needs review » Needs work

This needs a test written.

rmjiv’s picture

Confirmed bug! Subscribing

rich.yumul’s picture

subscribing.... I found this bug too...

t11o’s picture

subscribing

wonder95’s picture

This patch worked for me in 6.13. This definitely needs to be addressed in D7.

agentrickard’s picture

Version: 6.2 » 6.x-dev
Status: Needs work » Needs review
wonder95’s picture

Version: 6.x-dev » 7.x-dev

Changing to D7 so it can be fixed there, too.

Status: Needs review » Needs work

The last submitted patch failed testing.

deekayen’s picture

Status: Needs work » Needs review

bad trial run of new testing bot

wonder95’s picture

FileSize
1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-242324-20080422-1_0.patch. View

Re-uploading to have it tested again.

agentrickard’s picture

Patch looks clean. RTBC?

Status: Needs review » Needs work

The last submitted patch failed testing.

theemstra’s picture

Confirmed, in D6,
Subscribed.

theemstra’s picture

Status: Needs work » Needs review

#15: taxonomy-242324-20080422-1.patch queued for re-testing.

theemstra’s picture

Version: 7.x-dev » 6.9
Status: Needs review » Needs work

Still not working in 6.9, first patch failed, no effective results.
Same for second patch.

Crell’s picture

Version: 6.9 » 7.x-dev

This is still a bug in D7 so it has to get fixed there first.

theemstra’s picture

Ok.
I was looking, but didn' t find anything yet.

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
1.54 KB
PASSED: [[SimpleTest]]: [MySQL] 35,743 pass(es). View

Rerolled patch for 7.x.

delta’s picture

Version: 7.x-dev » 7.7

Same for me, i have a large vocabulary with few root term and under her, a long tree (more than 200 items)
when i come to the second pages, the page is empty, the pager was not.

with the patch proposed, the second page show the same term than the first page.

the only way i find for the moment is to count in a hook_taxonomy_term_presave()
the nb of term under the first root term, and then set the variable variable_set('taxonomy_terms_per_page_admin', $count + 20);

+ 20 ? if not count 20 more terms weird issue, like that works well... but it's a crap

so suscribing !

delta’s picture

sepehr.sadatifar’s picture

#2: taxonomy-242324-20080422-1.patch queued for re-testing.

xjm’s picture

Version: 7.7 » 8.x-dev
Issue tags: +Needs tests

We need to add a test for this that fails without the patch in #23, and passes with it. Before/after screenshots of the pager would also be helpful.

xjm’s picture

Marked #1300208: Taxonomy paging generates wrong pages as a duplicate of this issue.

xjm’s picture

Let's:

  1. Confirm whether this is reproducible on D8 HEAD. If it does:
  2. Write a test case that fails when the bug is present.
  3. Update the existing patch to D8 HEAD (maybe adding some code comments), and upload a test-only patch followed by the combined patch to expose the test failures and confirm that the patch fixes them.
  4. Provide before-and-after screenshots illustrating the fix.
perceptum’s picture

Status: Needs review » Needs work

Created 1000 new terms using devel (gent) on D8 HEAD install. This is still a problem in D8. The links for "next page" and "previous page" change the page, but "No terms available" message results. Selecting "page 2" in the pager is also broken. The first and last links however do work as expected.

BrockBoland’s picture

Needs issue summary

TimVS’s picture

FileSize
148.89 KB
255.1 KB

To re-produce issue:

* Create a new vocabulary (/admin/content/taxonomy/add/vocabulary)
* Add more terms than will fit onto the "list terms" page, this is 100
* Make sure that one parent has more than 100 siblings
* On page one everything is working (screenshot 1)
* Click on page 2 (or "next") on the "list terms" page
* It will go to the the next page but says "No terms available."(screenshot 2)

I`ll try to write an automated test so it can be reproduced.

kbasarab’s picture

Issue tags: -Needs issue summary update
FileSize
1.61 KB
PASSED: [[SimpleTest]]: [MySQL] 40,391 pass(es). View

Rerolls patch from #23 into Drupal 8.x Also editing issue summary.

kbasarab’s picture

Status: Needs work » Needs review

Needs review to ensure initial reroll didn't break anything.

estrejlau’s picture

With the patch in #23, I'm having the same problems as delta (#24). Also, 161 terms show up on the first page even though my variable taxonomy_terms_per_page_admin is set to the default, 100. Subscribing.

COBadger’s picture

FileSize
50.35 KB
48.73 KB

Tested patch #33 and it works for me.

Cottser’s picture

Issue tags: -Needs reroll

Updating tags, thanks @kbasarab!

ryanissamson’s picture

The #33 patch shows the same results on the second page rather than the empty page.

kbasarab’s picture

FileSize
24.07 KB
31.04 KB
46.7 KB
41.34 KB

ryanissamson: Yeah I thought the same thing. I'm actually doing some other testing now working on writing the tests for this. Seems as though when the term has more children then the page allows to show the items per page is not inherited.

Screenshots below:
Set to show only 9 terms per page via variable_set but this actually shows 10 terms per page (Separate issue)

Page 1: Shows A root term with two children and then our test term with 11 children.
Page 2: Show the same test term with 11 children but then continues on to show more terms
Page 3: Shows 10 terms as expected without any duplication as nothing on this page has more children than page needs.
Page 4: Final terms.

I'm just working on testing now. If someone wants to correct the other behavior. But it is better than before which showed zero terms on following page.

kbasarab’s picture

Issue tags: -Needs screenshots
FileSize
4.32 KB
PASSED: [[SimpleTest]]: [MySQL] 41,452 pass(es). View
2.72 KB
FAILED: [[SimpleTest]]: [MySQL] 41,433 pass(es), 19 fail(s), and 0 exception(s). View

First run at creating tests for this. I feel there is probably a better way of testing this but I'm open to suggestions.

The FAIL patch also serves as the interdiff between 33 and 40.

Also removing screenshots tag as there are quite a few screenshots showing issue on case now.

LinL’s picture

FileSize
4.34 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 242324-41.patch. Unable to apply patch. See the log in the details link for more information. View

Re-roll to use config('taxonomy.settings')->set instead of variable_set.

cgalli’s picture

FileSize
4.34 KB
FAILED: [[SimpleTest]]: [MySQL] 55,472 pass(es), 174 fail(s), and 570 exception(s). View

reroll of the patch, adapted to the chances since then.

Status: Needs review » Needs work
Issue tags: -Needs tests, -Novice, -Needs manual testing

The last submitted patch, 242324-42.patch, failed testing.

Kevin Morse’s picture

Status: Needs work » Needs review

#42: 242324-42.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +Novice, +Needs manual testing

The last submitted patch, 242324-42.patch, failed testing.

m.sant’s picture

I'm experiencing the bug in a D7 (7.22) installation with a large Taxonomy (9 pages in admin/structure/taxonomy).

Any news on this?

Should I use the patch provided in #23?

Cheers,
Marco

svenhoutmeyers’s picture

Same problem here with 7.22, and patch in #23 didn't work for me.

svenhoutmeyers’s picture

Issue summary: View changes

Issue Summary Initiative change.

prabeen.giri’s picture

41: 242324-41.patch queued for re-testing.

The last submitted patch, 41: 242324-41.patch, failed testing.

rychannel’s picture

FileSize
2.67 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,913 pass(es), 1 fail(s), and 0 exception(s). View

Rerolling the test to see if the test works on current 8.x head.

rychannel’s picture

Status: Needs work » Needs review

Setting to needs review to have the testbot run

rychannel’s picture

FileSize
2.67 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,068 pass(es), 1 fail(s), and 0 exception(s). View
390 bytes

Reroll of test to see if works on current current 8.x head PLUS removal of excessive newlines at end of testTaxonomyTermChildTerms function.

The last submitted patch, 50: 242324.50.tests-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 52: 242324.52.tests-only.patch, failed testing.

rychannel’s picture

Status: Needs work » Needs review
FileSize
2.68 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,369 pass(es), 41 fail(s), and 777 exception(s). View
663 bytes

I modified the config function call to call for \Drupal::config(). I should be able to spend more time on this tonight.

Setting to Needs Review so that the testbot will run.

Status: Needs review » Needs work

The last submitted patch, 55: 242324.55.tests-only.patch, failed testing.

rychannel’s picture

Status: Needs work » Needs review
FileSize
2.77 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,533 pass(es), 19 fail(s), and 0 exception(s). View
3.18 KB

This test patch should show that there is still some issue with taxonomy in D8. It will show that Page 1 works fine, Page 2 is empty, and Page 3 appears to work properly.

Once again setting to Needs Review to activate TestBot.

Status: Needs review » Needs work

The last submitted patch, 57: 242324.57.tests-only.patch, failed testing.

rychannel’s picture

Status: Needs work » Needs review
FileSize
4.58 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,354 pass(es). View

This patch *should* fix the issue. It passed my test script that I posted in #57. I'll let the TestBot run and see what he can find.

I adapted the code from the folks that posted previous patches. Their code wouldn't work in the current 8.0.x HEAD.

chx’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests, -Novice, -Needs manual testing

This looks great, thanks for the patch and the test!

At first I didn't understand the change from current-prev-next to indexing but the problem is that once prev "steps off the cliff" and moves the array pointer beyond the first element, next will not bring it back. So that change is good and I do not think it requires commenting; noone would want to change it back in such a convoluted piece of code, I am quite sure -- and the test would break if they would.

My second problem is that the test while very nice uses the UI to edit the terms. If you would add an optional $values array to createTerm() and then pass it to entity_create like entity_create('taxonomy_term', $values + array( this then you wouldn't need an additional (and very slow) step of using the UI to edit the term you just created. Setting it to needs work for this.

Now, this doesn't need a test any more and doesn't need manual testing thanks to the very nice test written.

rychannel’s picture

I'll see what I can do to modify the test code to work this way. All of the code was pulled from previous patches from other folks. I just modified it enough to make it work with current 8.0.x

Either they didn't know there was a better way or they were trying to simulate how the end user would have added terms.

rychannel’s picture

Status: Needs work » Needs review
FileSize
1.94 KB
3.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,334 pass(es), 19 fail(s), and 0 exception(s). View

This patch removes the use of the UI when adding Taxonomy Terms within the test. Hopefully this will increase the performance of the test. The createVocabulary function in TexonomyTestBase now accepts an optional $values array as well.

Status: Needs review » Needs work

The last submitted patch, 62: 242324.62.patch, failed testing.

Status: Needs work » Needs review

rychannel queued 62: 242324.62.patch for re-testing.

rychannel’s picture

shoot, I somehow mixed the fix in the patch. *sigh* I'll try to get it generated properly shortly.

rychannel’s picture

FileSize
5.34 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,180 pass(es). View

Alright folks, lets try this again. This includes the modified test and the fix.

The last submitted patch, 62: 242324.62.patch, failed testing.

rychannel’s picture

FileSize
2.82 KB
5.29 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,386 pass(es). View

I cleaned up the code in this patch to meet Drupal Coding Standards. See interdiff for lines affected.

Also removed a couple lines of redundant code that specified the term had no parent if it wasn't one of the first 12 terms. Parent by default has a value of 0.

rychannel’s picture

FileSize
5.29 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,372 pass(es). View
2.82 KB

I had an issue in the last upload and then I found more areas that needed cleaned up.

chx’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
5.85 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 242324_70.patch. Unable to apply patch. See the log in the details link for more information. View
1.21 KB

This is ready, thanks much for the fixes, looking really nice.

createTerm() was missing doxygen and typehints, I added that. But that's not a significant enough change to wait for another reviewer, I felt OK with RTBC'ing this.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Confirmed that the test fails without the patch. Nice work.

Committed 9bc009b and pushed to 8.x. Thanks!

  • alexpott committed 9bc009b on 8.0.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...

Status: Fixed » Closed (fixed)

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

kenorb’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Closed (fixed) » Needs review

What about 7.x? It is still happening in 7.32. Patch #23 or any other doesn't seems to be committed.

chx’s picture

FileSize
5.11 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 242324_75-taxonomy-admin-tree-broken.patch. Unable to apply patch. See the log in the details link for more information. View

I deemed the code unfixable, wiped it and wrote a new one. This code will always display at least taxonomy_terms_per_page_admin and a new page will always start with a top level element.

Status: Needs review » Needs work

The last submitted patch, 75: 242324_75-taxonomy-admin-tree-broken.patch, failed testing.

chx’s picture

Status: Needs work » Needs review
FileSize
5.08 KB
PASSED: [[SimpleTest]]: [MySQL] 41,106 pass(es). View
chx’s picture

Version: 7.x-dev » 8.0.x-dev
FileSize
13.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 242324_78.patch. Unable to apply patch. See the log in the details link for more information. View

I do not even know what HEAD is supposed to do. The test passes but looking back either I was much smarter in July and comprehended the code or I haven't comprehended the problem and haven't considered the true extent of it. This version does away with the forward and backward jumps as it's very confusing. Very simple code. Works well.

chx’s picture

Issue summary: View changes
chx’s picture

To clarify: the current behavior:

Terms from previous and next pages are shown if the term tree would have been cut in the middle.

This is incredibly confusing to clients. The code to do this is incredibly confusing to me. Both can be eliminated by this simple rule:

Do not cut trees in the middle. Once a tree starts, it displays on a page.

This makes the setting a minimum instead of a hard value but I think that's less of a problem. It makes much easier to administer trees as well. If you have so many terms the browser kneels from this then use some other UI. Paginating a tree is utterly and completely hopeless user experience wise.

balagan’s picture

For me no pager shows up at all.
The steps in problem/motivation need to be updated, when setting terms_per_page_admin = 3, there is no need for 100 terms to have 1 parent.

balagan’s picture

Issue summary: View changes
balagan’s picture

Issue summary: View changes
balagan’s picture

Issue summary: View changes

balagan queued 78: 242324_78.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 78: 242324_78.patch, failed testing.

balagan’s picture

FileSize
13.12 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 242324_87.patch. Unable to apply patch. See the log in the details link for more information. View

Rerolled the patch in #78. The core/modules/taxonomy/css/taxonomy.module.css file was renamed to core/modules/taxonomy/css/taxonomy.theme.css, but its contents were the same, so I deleted that.
The line referring to the above mentioned css file was changed in core/modules/taxonomy/taxonomy.libraries.yml file to its new name, so I bravely deleted this file too.

balagan’s picture

Status: Needs work » Needs review
balagan’s picture

I confirm that applying the patch in #87 all children terms appear on the page, and the next page starts with the next parent item.

pjonckiere’s picture

Got here from #2425535: No pager shows on taxonomy overview pages, which I just RTBC'd. This will resolve the issue that no pager is displayed as mentioned in #81.

The patch in this ticket looks good and has good coverage. It will need manual testing once the patch above gets in.

xjm’s picture

Is this still an issue in 8.0.x? When testing #2425535: No pager shows on taxonomy overview pages I did not encounter this bug.

balagan’s picture

@xjm: Did you use the steps mentioned in this summary?

David_Rothstein’s picture

Issue tags: +needs backport to D7

rychannel queued 70: 242324_70.patch for re-testing.

The last submitted patch, 70: 242324_70.patch, failed testing.

chx’s picture

Be careful: if you tree grows over 250 items then the default 1000 POST vars will make Drupal silently disregard the form submission since the form_id is after the tree and each tree item is 4 variables.

chx’s picture

FileSize
8.47 KB

Newer D7 patch.

douggreen’s picture

FileSize
8.44 KB

minor change to above D7 patch from @chx, just because our patch system can't apply this properly

mgifford queued 87: 242324_87.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 87: 242324_87.patch, failed testing.

  • alexpott committed 9bc009b on 8.1.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...
gambry’s picture

Patch on #98 works on v7.41 (the one on #97 has a reference to a docroot/ folder).

Issue is pretty bad. I thought terms where missing and I was going to delete and recreate the vocabulary (198 terms).
It should be included in D7 ASAP.

sukh.singh’s picture

Thanks a lot @chx and @douggreen for the patch 242324-98-do-not-test.patch. This works for me For D7.

gambry’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Needs work » Reviewed & tested by the community

Patched committed to 8.0.x (#72) and 8.1.x (#101), so changing version and status as apparently only 7.x commits is needed.

David_Rothstein’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Reviewed & tested by the community » Needs review

The latest Drupal 7 patch is very different from the patch that was committed to Drupal 8, and doesn't have tests yet (in fact it's even marked "do-not-test").

I think @chx moved this back to Drupal 8 based on the idea that the Drupal 8 fix wasn't good (see #80).

However if what went into Drupal 8 is at least an improvement over the previous situation (?), we should backport that to Drupal 7 here, and then move the further improvements to a followup issue instead.

Moving back to "needs review" for feedback on that.

balagan’s picture

This was already commited 8.1.x, see #101.

gambry’s picture

@David_Rothstein I'm not really sure where @chx moved this back to Drupal 8 - apologize for that -, but I do see his concerns on #80 are issued on patch from @balagan at #87 and committed to 8.1.x at #101.

Also D7 patch from @chx at #97 covers these concerns as well.

I understand no automatic tests have ran against the D7 patch due the -do-not-test suffix, but @dougreen, @sukh.singh and I have tested the patch manually and that's why I set the issue as Reviewed ant tested by the community.

However if what went into Drupal 8 is at least an improvement over the previous situation (?), we should backport that to Drupal 7 here, and then move the further improvements to a followup issue instead.

I'm not sure what has been committed to 8.0.x, but both commits to 8.1.x and patch for 7.x covers @chx concerns, are definitely a big improvement and fix a dangerous bug (see #102 about why I consider it dangerous).

Just my thoughts, let's collect more feedbacks.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • alexpott committed 9bc009b on 8.3.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...

  • alexpott committed 9bc009b on 8.3.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Alienpruts’s picture

Hello,

not entirely sure if this is related or not (apologies if not), but I think that the changes implemented here have led to an unintended side effect : parents with a lot of children tend to break pagination of the Taxonomy Overview Terms page.

If interested, https://www.drupal.org/node/2806551 .

Tnx,

  • alexpott committed 9bc009b on 8.4.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...

  • alexpott committed 9bc009b on 8.4.x
    Issue #242324 by rychannel, kbasarab, chx, cgalli, LinL, pfrenssen,...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.