Problem/Motivation

In #3215784: [META] Fix up topics to use new help_route_link function, the patch was too large and too burdensome to review, so the issue was re-scoped and child issues created. The patches in #58 (9.5.x) and #61 (10.1.x) can be used as a reference.

On #3090659: Make a way for help topics to generate links only if they work and are accessible, we've added two new functions for making safe links with access checks in help topics.
Related change record is https://www.drupal.org/node/3192582

As an example, this is how the block.place.html.twig file was updated in the patch on that issue (that was the only production topic that was updated):

-{% set layout_url = render_var(url('block.admin_display')) %}
+{% set layout_link_text %}
+{% trans %}Block layout{% endtrans %}
+{% endset %}
+{% set layout_link = render_var(help_route_link(layout_link_text, 'block.admin_display')) %}
 
...

-  <li>{% trans %}In the <em>Manage</em> administrative menu, navigate to <em>Structure</em> &gt; <a href="{{ layout_url }}"><em>Block layout</em></a>.{% endtrans %}</li>
+  <li>{% trans %}In the <em>Manage</em> administrative menu, navigate to <em>Structure</em> &gt; {{ layout_link }}.{% endtrans %}</li>

...

-  <li>{% trans %}Configure the block and click <em>Save block</em>; see <a href="{{ configure }}">Configuring a previously-placed block</a> for configuration details.{% endtrans %}</li>
+  <li>{% trans %}Configure the block and click <em>Save block</em>; see {{ configure_topic }} for configuration details.{% endtrans %}</li>

Steps to reproduce

N/A

Proposed resolution

1. Update breadcrumb links in steps or standalone "navigate to X" links in the following help topics:

  • core/modules/help_topics/help_topics/contact.adding_fields.html.twig
  • core/modules/help_topics/help_topics/contact.configuring_personal.html.twig
  • core/modules/help_topics/help_topics/contact.creating.html.twig
  • core/modules/help_topics/help_topics/contact.setting_default.html.twig
  • core/modules/help_topics/help_topics/content_translation.overview.html.twig
  • core/modules/help_topics/help_topics/core.cron.html.twig
  • core/modules/help_topics/help_topics/editor.overview.html.twig
  • core/modules/help_topics/help_topics/field_ui.add_field.html.twig
  • core/modules/help_topics/help_topics/field_ui.manage_display.html.twig
  • core/modules/help_topics/help_topics/field_ui.manage_form.html.twig
  • core/modules/help_topics/help_topics/field_ui.reference_field.html.twig
  • core/modules/help_topics/help_topics/filter.overview.html.twig

2. Ensure that no other breadcrumb links need to be updated in contact, content_translation, cron, editor, field_ui, filter help topics. (Search for usages of render_var(url( or uses of HTML anchor tags (for breadcrumbs only!) in those help topics.)

3. Please do NOT include any sentence-embedded links in the patch, as these will be reviewed in a separate issue with respect to translatability.

Remaining tasks

Patch, review, commit.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

Not necessary. This is an experimental module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Amber Himes Matz created an issue. See original summary.

_shY’s picture

Status: Active » Needs review
FileSize
25.83 KB

Updated breadcrumbs according to the main patch.

smustgrave’s picture

FileSize
0 bytes
25.88 KB

Status: Needs review » Needs work

The last submitted patch, 3: 3307692-3.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Needs review
FileSize
1.59 KB
25.91 KB

Fixed an issue in field_ui.reference_field

Status: Needs review » Needs work

The last submitted patch, 5: 3307692-5.patch, failed testing. View results

Amber Himes Matz’s picture

Thank you for the patches @_shY and @smustgrave!

+++ b/core/modules/help_topics/help_topics/field_ui.reference_field.html.twig
@@ -7,12 +7,13 @@ related:
+  <li>{% trans %}Navigate to the page for managing the entity sub-type you want to add the field to. For example, to add a field to a content type, in the <em>Manage</em> administrative menu, navigate to <em>Structure</em> &gt; <a href="{{ content_types_link }}"><em>Content types</em></a>.{% endtrans %}</li>

Here we want to replace the entire <a> tag with the Twig variable we set for the link.

smustgrave’s picture

Status: Needs work » Needs review
FileSize
43.25 KB
26.02 KB

Sorry about that!

Amber Himes Matz’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the updated patch, @smustgrave!

I tested using the following steps:

  1. Checked that the patch covered all the files listed in the issue summary.
  2. Installed Drupal 9.5.x
  3. Applied the patch
  4. (Re-) Installed Help Topics (and any other modules with help topics)
  5. Cleared/Rebuilt Cache
  6. Ran cron (to update the search index and use the help search to test each updated topic)
  7. For each modified help topic file, reviewed the code to ensure the conversion was done correctly.
  8. For each modified help topic, searched for the topic using the search on /admin/help
  9. Read the help topic to ensure HTML was rendered correctly.
  10. Tested each updated route link.
  11. Ensured that the text of the link matched the menu item or the page title.

For each of the help topics in this patch, all of my tests passed. Because of that, the patch in #8 looks good to me.

Note: The link text in content_translation.overview.html.twig, "Content language and translation", is a string that matches the link text on the "Regional and language" settings page. Which, if you are following the steps as described, is the link text you would need to look for and click on. But the page itself is titled "Content language", so there might be confusion when reviewing this because the 2 are different. But we are not introducing a new string here, just using the link text as it appears on the "Regional and language" configuration page.

Amber Himes Matz’s picture

Status: Reviewed & tested by the community » Needs review

Added a test run for 10.1.x. We need to test this patch in 10.1.x to see if it applies, and create a separate patch for 10.1.x if it doesn't.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Moving back to RTBC as the 10.1.x based

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 1b1b3ecdcc to 10.1.x and ab1a77d697 to 10.0.x and 6370fe1fc3 to 9.5.x. Thanks!

  • alexpott committed 1b1b3ec on 10.1.x
    Issue #3307692 by smustgrave, _shY, Amber Himes Matz: Update breadcrumbs...

  • alexpott committed ab1a77d on 10.0.x
    Issue #3307692 by smustgrave, _shY, Amber Himes Matz: Update breadcrumbs...

  • alexpott committed 6370fe1 on 9.5.x
    Issue #3307692 by smustgrave, _shY, Amber Himes Matz: Update breadcrumbs...

Status: Fixed » Closed (fixed)

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