Page title with HTML tags shows HTML tags in shortcut label in the shortcut toolbar tray

I have tested the patch, but it is not giving the desired output.

This is not a correct fix and we need some tests here.

Here we go. Red and green patch.

Before

After

We need to fix that in 8.0.x as well.

Hi Team,

I feel this is invalid request and its working as the way node title works.Since we have default styling for links on core, need to have any tag over the links/title

Node screen [d8nodescreen.png]

Let me your thought.

I tested patch provided in #9 strip_html_tags_from-2672668-9-pass.patch This patch is fixing this issue. I followed below steps in fresh drupal 8.2.0-dev installation.

1. Clone drupal 8.2-x branch.
2. Do a git pull make sure everything is up to date.
3. Follow installation process.
4. Goto main menu edit url /admin/structure/menu/manage/main where title is dynamic.
5. Add this page into shortcut this will show you shortcut with html.
   
6. Apply this patch.
7. Drush rebuild cache with drush cr
8. Goto same page and remove shortcut and add shortcut again.
9. Shortcut with html is gone now. Only shortcut title is show as expected.
   

So RTBC then?

Yes, Moving this to RTBC, I can confirm that this patch fix this issue. Feel free to move back in "Need Review" if someone still find any issue with this.

This is a bug report so it should be filed against 8.1.x. Test fails seem unrelated so retesting and setting it back to RTBC.

#24 is just a reroll so back to RTBC.

1. +++ b/core/modules/shortcut/shortcut.module
   @@ -311,7 +311,8 @@ function shortcut_preprocess_page_title(&$variables) {
   +    // Strip the html tags from the title.
   

   Nit: s/the html/HTML/

2. +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php
   @@ -185,6 +187,23 @@ public function testShortcutQuickLink() {
   +    // Update router to reflect newly installed module.
   +    \Drupal::service('router.builder')->rebuild();
   

   This should not be necessary?

3. +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php
   @@ -185,6 +187,23 @@ public function testShortcutQuickLink() {
   +    // Test page with html tags in title.
   

   Nit: s/html/HTML/

4. +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php
   @@ -185,6 +187,23 @@ public function testShortcutQuickLink() {
   +    // Add Shortcut for block content type edit form.
   

   IMO clearer:

   // Add shortcut to this page.
   

5. +++ b/core/modules/shortcut/src/Tests/ShortcutLinksTest.php
   @@ -185,6 +187,23 @@ public function testShortcutQuickLink() {
   +    $this->assertRaw(new FormattableMarkup('Added a shortcut for %title.', ['%title' => trim(strip_tags($page_title))]));
   +
      }
   

   Nit: extraneous newline.

Fixed #26.

Much better!

Looks like very similar to #2478907: Shortcuts names for node view routes are not properly sanitized (using quick link). If you want prefer this issue and mark #2478907: Shortcuts names for node view routes are not properly sanitized (using quick link) as duplicated can we add the test coverage produced in that issue?

Random fail.

iCNW for #29 - please also mark the other issue duplicate and we'll transfer commit credits from their prior to commit.

Here we go.

Yup, looks good to me.

Adding credit from the other issue.

Committed and pushed d963647 to 8.3.x and 3c355f8 to 8.2.x. Thanks!