Shortcut link erroneously transfers markup from the page title to the shortcut name [#2358353]

Problem/Motivation

shortcut_preprocess_page() has this code:

$query = array(
  'link' => $link,
  'name' => $variables['title'],
);

However, $variables['title'] might have markup in it. For example, on a node page, it has a wrapping span to support quickedit module. So, if you are using a theme that enables the shortcut link on a node page, then when you click the link to add the shortcut, it adds it with a shortcut name containing that markup. That is not desired, because shortcut listings (e.g., the Shortcuts tab in the toolbar) list shortcuts as plain text, so any markup in their name gets double escaped.

Proposed resolution

Option 1

Change the above to strip_tags($variables['title']).

Option 2

Don't use $variables['title'] at all, but use the route's title instead. e.g., \Drupal::service('title_resolver')->getTitle(\Drupal::request(), \Drupal::routeMatch()->getRouteObject()). A variant of this option might also address #686440: Shortcuts that point to dynamic page titles don't automatically update.

Remaining tasks

Decide on option. Write patch with fix + test.

User interface changes

API changes

Comments

Comment #1

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 20 October 2014 at 15:30

PathBasedBreadcrumbBuilder::build() also uses option 2.

Comment #2

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 20 October 2014 at 15:37

There's also a great potential for inconsistencies (i.e. shortcut title != title that the user sees on the page)… but we already have inconsistencies:

To top it off, the path-based breadcrumb builder will just look at every single path component along the way… including those that don't represent a valid path! E.g. /nl/node/1/translations/add/en/nl, for which _title = 'Add' is defined, but it's dynamically overridden using #title. The breadcrumb builder then gets the titles for /nl/node/1/translations/add/en and /nl/node/1/translations/add, which is 'Add' according to the static title, but in fact neither of those paths are *valid*! They both trigger exceptions. So we get a breadcrumb like Home > NODE TITLE > Translations > Add > Add, but neither of the two last links work, and this fact is subtly hidden, in part thanks to the combination of _title and #title.

(From #2359901: Discourage $main_content['#title'] in favor of route titles and title callbacks.)

Comment #3

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran as a volunteer commented 3 April 2016 at 21:29

I think this is a duplicate of #2672668: Page title with HTML tags shows HTML tags in shortcut label in the shortcut toolbar tray. #2672668 has a patch and waiting for review.

Comment #4

3 April 2016 at 21:29

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.

Comment #5

3 April 2016 at 21:29

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.