Add missing vocabulary to fix term edit form breadcrumbs

Features get added to latest version first and needs review indicates a patch exists.

This is a very useful feature to have. I have had similar problems in the past.
From the screenshot Breadcumb should also say Home >> Fruits >> Banana

@@ -34,7 +34,7 @@
 /**
  * Minimum supported version of PHP.
  */
-const DRUPAL_MINIMUM_PHP = '5.3.5';
+const DRUPAL_MINIMUM_PHP = '5.3.3';

Why is the minimum php version being changed in this patch?

'#markup' => '<b>' . t('Vocabulary') . ':</b>&nbsp;' . l(t($vocabulary->name), 'admin/structure/taxonomy/' . $vocabulary->id()),

Variables should not go through t(). Similar issues in the breadcrumb code.

Looks good and a very handy feature. Shouldn't <strong> be used instead of <b> in #markup?

This looks good to me

This looks like a useful UX enhancement. I'm re-categorizing as a task rather than a feature though, because it's pretty straight-forward.

However, we need automated tests that show the functionality works as expected. See http://drupal.org/simpletest or drop by core mentoring hours if you need a hand!

Re rolled patch with test included.

I ran all TAXONOMY TERM FUNCTIONS AND FORMS tests which pass.

$this->assertLink(t('Add term'),0,'Vocabulary link works.');

Does not meet drupal coding standards also no need for a message to be supplied just let the assert return the default message.

patch without a message

Looks good.

It works because it is clicking the link and ensuring a page is loaded with the link

$this->clickLink($this->vocabulary->name);

Your test works equally as well as it is checking for the presence of the link and not following it. Either version works assuming this comes back green it is RTBC. And nice catch on the white space I missed those before when doing an eyeball review :)

Please correct me if I'm wrong but is it not better to test if link is actually working and it's not broken (by going to vocabulary page and testing content there) than just simply test presence of the link on term page?

I prefer the patch in #24. It does what #22 does, but it also tests the link is working.

You say:
Clicking on the link to the vocabulary will keep you on the same page as the link is on the LIST tab
$this->clickLink($this->vocabulary->name);

I see the link on the EDIT page. Clicking the link takes you to the term list. The test in #24 should be adopted.

#24: show_vocabulary-1905370-24.patch queued for re-testing.

re-rolled patch #24

I also think that test in #30 is better, it definitely checks the link on term page.

#30: show_vocabulary-1905370-30.patch queued for re-testing.

Okay, it won't let me embed the screenshots for some reason, but they're in #0 (before) and #1 (after).

You know...

Looking at this again after some time away, I'm wondering if we want to do exactly this. It seems to introduce inconsistency with what we do at e.g. menu links, which are also hierarchical. There, the method of informing you what menu you're editing is merely by changing the breadcrumb to include it: Home >> Administration >> Structure >> Menus >> Main navigation

I'm actually thinking it would be better if we were just to do that much of the patch, and not also the part of entering a new form element above Name, which is generally the very first element in a form, so it's a bit odd for it not to be here.

Tagging for the UX team to chime in.

The changes look good, but one of the sets of fails is this which looks suspiciously related:

    // Edit the term.
    $this->drupalGet('taxonomy/term/' . $term->id() . '/edit');
    $this->assertResponse(200);
    $this->assertText($edit['name'], 'Edit taxonomy term form opened successfully.');

It is pretty clearly related to the taxonomy term edit page, which looks to return a 200 response but not have the term there? This may be due to prior tests finding it in the breacrumb (instead of the form?). If you run the test locally, you will get debug page output for these fails so you can check what is going on there.

Great, so you added the link to the term, so now it works again :) Looks good to me. This should improve the velocity of taxonomy administration a lot.

This is a very useful addition, indeed. The patch applies and the breadcrumb works like a charm on the term edit page. However when adding a term, the breadcrumb is different. For example when clicking the vocabulary name, I end up on the vocabulary edit page.

It would be great when the term-edit and the term-add breadcrumbs would work the same way.

Needs a reroll after #2292025: Convert BreadcrumbBuilderInterface applies() and build() to use RouteMatch.

@marcelovani: Is there any technical reasons against #50?

I'm willing to continue reviewing / RTBC this. I leave the decision whether or not to include #50 to the person working on this patch and to the person who eventually will commit it. The patch already has been rejected once because of consistency reasons (see #36), therefore I think it would be smart to at least give it a try. If it is easy then do it, if not then let's open a follow-up.

Modifying the breadcrumb is not considered an API change, so no need to fear rejection because of that. The issue summary needs an update anyway to reflect #37.

I am not 100% sure whether this patch can be included in 8.0.x based on Beta changes policy. I think it should be possible because it was RTBC several times way before the beta deadline.

I guess the issue which marcelovani is talking in #51 has a patch already .That can be found on https://www.drupal.org/node/2400543

From #50

This is a very useful addition, indeed. The patch applies and the breadcrumb works like a charm on the term edit page. However when adding a term, the breadcrumb is different. For example when clicking the vocabulary name, I end up on the vocabulary edit page.

This issue will be irrelevant once #1340500: Merge "list terms" page into "edit vocabulary" page is fixed.

Also see #2428261: Breadcrumbs on Taxonomy add page take you back to the Edit not List page for #50.

Changes of TermBreadcrumbBuilder.php moved to PSR-4 file location(Check interdiff).

Patch rerolled.

Could the link to the vocabulary be shown in the description of the field? More compact and closer to the term field itself:

(The other arrow is for pointing out that yes, the breadcrumb needs fixing too)

Regarding #66 I think it is a good idea to put some explanatory text somewhere, but the text underneath the field is by default reserved for the field's description/help text and that is totally up to the site-builder to set that.
I would suggest that it could very well done also in this issue: #2596207: More specific titles for term and vocabulary edit forms where we could set the title lieke so:

Edit term "Apple" in "Fruits" vocabulary

Then it would be not so bad that someone could override/change the description

Sounds like it's necessary to choose between this issue and #2596207: More specific titles for term and vocabulary edit forms? It is hard to imagine doing both of them.

Well, I take that back - to the extent this issue is about fixing the confusing breadcrumbs, it makes sense to do no matter what. But in that case it should probably be retitled.

Per #71