Problem/Motivation

Shortcuts to pages generate by views are not recognized as added to the shortcut set and and this allows you to save several times a shortcut for the same page using quick link.
This seems to be caused by the check to determine the operation for the current page in shortcut_preprocess_page():324. For each shortcut is checked if the parameters of the current route are the same as those saved for the shortcut.. BUT route parameters are never saved for shortcuts only the uri



Proposed resolution

Remove route parameters check from shortcut_preprocess_page()

Remaining tasks

Make a patch.

User interface changes

none

API changes

none

CommentFileSizeAuthor
#3 shortcuts_to_pages-2478151-3.patch2.37 KBwillzyx
#3 shortcuts_to_pages-2478151-3-test-only.patch1.44 KBwillzyx
#1 shortcuts_to_pages-2478151-1.patch950 byteswillzyx
shortcut-views.jpg44.6 KBwillzyx

Comments

willzyx’s picture

willzyx commented
Title: Shortcuts to pages generate by views are not recognized as added to the shortcutset and are being added multiple times » Shortcuts to pages generated by views are not recognized as added to the shortcutset and are being added multiple times
StatusFileSize
new shortcuts_to_pages-2478151-1.patch950 bytes
willzyx’s picture

willzyx commented
Status: Active » Needs review
willzyx’s picture

willzyx commented
StatusFileSize
new shortcuts_to_pages-2478151-3-test-only.patch1.44 KB
new shortcuts_to_pages-2478151-3.patch2.37 KB

Added test

The last submitted patch, 3: shortcuts_to_pages-2478151-3-test-only.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Patch looks good fix looks good.
Can we add a test for node view routes? i.e. node/1 and node/2.

willzyx’s picture

willzyx commented

sure we can.. but from what I remember the quick link does not appear on node view routes on default theme, so how should we build the test?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Maybe enable seven by default.

willzyx’s picture

willzyx commented

Tests on node view routes show that the name of the shortcut is not properly escaped. This should be a different issue. Opened #2478907: Shortcuts names for node view routes are not properly sanitized (using quick link)
The node view routes tests will be developed there

willzyx’s picture

willzyx commented

@jibran since patch looks good, fix looks good and tests for node view routes will be developed in #2478907: Shortcuts names for node view routes are not properly sanitized (using quick link) can we move forward and mark this RTBC?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Yeah sure.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed d2c38c4 and pushed to 8.0.x. Thanks!

  • alexpott committed d2c38c4 on 8.0.x 
    Issue #2478151 by willzyx: Shortcuts to pages generated by views are not...

Status: Fixed » Closed (fixed)

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

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

This introduced #2511024: Can't add multiple content types to shortcuts. Reverting this locally fixed the issue for me.