Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Open this issue after #2478151: Shortcuts to pages generated by views are not recognized as added to the shortcutset and are being added multiple times
Shortcuts names for node view routes are not properly sanitized using quick link.
In the pictures below you can see what happens trying to add a node in the shorcut set
Proposed resolution
Properly escape shortcuts name in shortcut_preprocess_page()
Remaining tasks
Make a patch.
User interface changes
none
API changes
none
Comment | File | Size | Author |
---|---|---|---|
#24 | 2478907-24.patch | 4.94 KB | lokapujya |
#20 | interdiff.txt | 1005 bytes | willzyx |
#20 | shortcuts_names_for-2478907-20.patch | 4.79 KB | willzyx |
#13 | shortcuts_names_for-2478907-13.patch | 4.37 KB | willzyx |
#13 | interdiff-5-13.txt | 4.69 KB | willzyx |
Comments
Comment #1
willzyx CreditAttribution: willzyx commentedComment #2
willzyx CreditAttribution: willzyx commentedComment #5
willzyx CreditAttribution: willzyx commentedsorry.. I didn't realize that to make this patch work is necessary that the changes in #2478151: Shortcuts to pages generated by views are not recognized as added to the shortcutset and are being added multiple times are committed.
Comment #6
willzyx CreditAttribution: willzyx commentedrerolled after #2478151: Shortcuts to pages generated by views are not recognized as added to the shortcutset and are being added multiple times
Comment #7
jibranWhy not just use check_plain?
Comment #8
willzyx CreditAttribution: willzyx commentedbecause check_plain doesn't remove markup inserted by the quick edit module (see IS)... but we can probably find a better solution
and also for semplicity we should create a function (::assertShortcutQuickLink()?) for test quick link
Comment #12
jibran+1 to #8 so NW
Comment #13
willzyx CreditAttribution: willzyx commentedAdded ::assertShortcutQuickLink() util method.
Looking at #5
'name' => trim(strip_tags($variables['title']))
is not a clean solution. We could usetitle_resolver
to retrieve the original title of the page.Comment #14
jibranOh much better now. Let's wait for the tests. Can you please upload the new after screenshot?
Comment #16
willzyx CreditAttribution: willzyx commentedThis is weird.. it seems
title_resolver
not work at all for pages generated by views.@jibran should we open another issue for this?
Comment #17
dawehnerbut don't we want to use the actual title which was part of the page? the docs doesn't make that parts clear for me, at least.
Comment #18
willzyx CreditAttribution: willzyx commentedfor now the problem related to the solution used in patch seems to be that
title_resolver
not work at all for pages generated by viewsdawehner look at the IS. If we use $variables['title'] for node view routes, the markup created by quickedit that wraps title became part of the shortcut name and this is wrong. Of course documentation can be improved, suggestions are welcome
Comment #19
willzyx CreditAttribution: willzyx commentedLooking at #2359901: Discourage $main_content['#title'] in favor of route titles and title callbacks and #2264043: Add back a simple page title getter the
title_resolver
approach is not applicable with the current API.The only alternative I see feasible is use
strip_tags
but I do not think is the right approachAny ideas?
Comment #20
willzyx CreditAttribution: willzyx commentedEnded up with a mixed approach: Use title from
title_resolver
when available$variables['title']
otherwise.. don't think it is the optimal solutionComment #21
jibranComment #23
jibranComment #24
lokapujyaRemoved the SafeMarkup::checkPlain() because it's in a preprocess function.
Comment #26
catchI think this should probably just use strip_tags() on $variables['title']?
Comment #27
jibranMoved tests to #2672668: Page title with HTML tags shows HTML tags in shortcut label in the shortcut toolbar tray.