Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect parsing of slug links inside Tip blocks #14

Closed
attilaantal opened this issue Nov 6, 2018 · 17 comments · Fixed by #182
Closed

Incorrect parsing of slug links inside Tip blocks #14

attilaantal opened this issue Nov 6, 2018 · 17 comments · Fixed by #182
Assignees
Labels
bug Something isn't working est: low Estimated for 1-2 dev days sev: high Impacts all documentations sites

Comments

@attilaantal
Copy link
Contributor

Description
Slug Links in Tip blocks are not interpreted/parsed correctly, hence the generated URL is invalid. Issue appears on multiple pages in the documentation.

To Reproduce
Visit any of the following pages, find the Tips that contain links and inspect the item.
Batch Editing Overview
Batch Editing Client-Side API
Data Editing - Custom Edit Forms

Additional context
Probably the entire Documentation is affected, where slug links are included in Tip blocks.

@imtodor
Copy link
Collaborator

imtodor commented Jan 9, 2019

Works fine with docs.telerik.com/devtools/justmock/getting-started/quick-start (link to the article - https://github.com/telerik/justmock-docs/edit/master/getting-started/quick-start.md).

Can you check whether the slugs exist?

@imtodor imtodor added the not reproducible Cannot reproduce it label Jan 9, 2019
@marin-bratanov
Copy link
Contributor

The JustMock article does not have the custom alert blocks from the ajax docs. My best guess is that the liquid tags within custom blocks are not evaluated at all. I think the fix we used to have for this was to make sure the custom blocks parsers either ran first, or ran last, perhaps Ianko Dzhemerenov will remember more, he wrote them.

The issue still manifests in the ajax docs, here is another article (see the Which File Do I Need to Install link in the second tip): https://docs.telerik.com/devtools/aspnet-ajax/getting-started/first-steps#first-steps-with-ui-for-aspnet-ajax.

Sample markup that causes the problem

>tip The following article can help you choose the installation type that is most suitable for your needs and preferences: [Which File Do I Need to Install]({%slug installation/which-file-do-i-need-to-install%}).

Resulting link https://docs.telerik.com/devtools/aspnet-ajax/getting-started/%7B%slug%20installation/which-file-do-i-need-to-install%%7D

Correct slug is present in the article: https://github.com/telerik/ajax-docs/blob/master/installation/which-file-do-i-need-to-install.md

@imtodor
Copy link
Collaborator

imtodor commented Feb 21, 2019

Here is another example for working slug in a custom alert block - https://docs.telerik.com/devtools/wpf/controls/radbulletgraph/radbulletgraph_overview

@marin-bratanov
Copy link
Contributor

marin-bratanov commented Mar 9, 2019

Here is another example of it not working in a custom alert block (see the second block): http://kendobuild-staging.openstack.progress.com/blazor/components/window/size

The problem comes from the / symbols. The ajax repo (and now the blazor repo, because it is me writing it) use those to match the paths in the slugs. The WPF repo uses dashes. I tried changing the slashes to dashes in the article above and it worked. Is there a way to keep using slashes? Even if I can rework the blazor repo (only 20ish articles at the moment), the ajax repo has well over 6000 so changing that is not feasible.

@eyupyusein
Copy link

@pepinho24
Copy link
Collaborator

pepinho24 commented Oct 14, 2019

One more not parsed slug in https://docs.telerik.com/devtools/aspnet-ajax/controls/htmlchart/appearance-and-styling/legend-settings the Visual Templates link in the tip down in the Appearance section

And one more with another type of note: https://docs.telerik.com/devtools/aspnet-ajax/controls/treeview/checkboxes/tri-state/tri-state-checkboxes-overview

@bsivanov
Copy link
Member

bsivanov commented Nov 13, 2019

We isolated that all blocks except the default one (> or >warning) are breaking the slugs.

@imtodor imtodor added bug Something isn't working and removed not reproducible Cannot reproduce it labels Nov 13, 2019
@dessyordanova
Copy link
Contributor

Please refer to the following help article and have a look at the first 3 important fields: https://devdocs.telerik.com/devtools/winforms/controls/richtextbox/overview
It seems that a slug with a slash in it breaks the link.

@marin-bratanov
Copy link
Contributor

marin-bratanov commented Jan 23, 2020

Yes, it's the slash in a slug in a > block #14 (comment) . I've started using dashes even though I have >100 articles with slashes already. Dashes are ok.

@dessyordanova
Copy link
Contributor

Yes, the slashes are OK. But we have quite a huge number of articles with slashes in the slug.

@marin-bratanov
Copy link
Contributor

oh, yes, in AJAX where I first hit this we have thousands (probably > 7000), in Blazor I have about 120, I think.

@imtodor
Copy link
Collaborator

imtodor commented Jan 23, 2020

Substituting slashes with dashes is easy, nevertheless the count of the slugs...Find/Replace with a regex will do the job.

@dessyordanova
Copy link
Contributor

But this will require redirects which are not always so easy to implement as it sounds.

@imtodor
Copy link
Collaborator

imtodor commented Jan 23, 2020

But this will require redirects which are not always so easy to implement as it sounds.

AFAIK, a slug is something like a 'key' for a given article? Why the redirect?

@dessyordanova
Copy link
Contributor

I suppose because, on the website, the real URL (containing the slash) will be stored in the page, not the slug. If we change the slug, wouldn't it change the URL as well? Maybe I am wrong?

@imtodor imtodor closed this as completed Jan 23, 2020
@imtodor imtodor reopened this Jan 23, 2020
@marin-bratanov
Copy link
Contributor

The slug is an internal ID, it does not pertain to the actual URL. The file name determines the URL

@pepinho24
Copy link
Collaborator

pepinho24 commented Nov 8, 2021

The issue seems to be related to the RegEx finding the slug inside the alert. It currently searches for "\w" and a dash:

A potential fix is to enhance the regex to include some of the more common characters as well /.*?(%7[Bb]%slug%20([0-9a-zA-Z_\-\(\)\*\.\/\,\%\'\?\:]+)%{2}7[Dd])/

This covers all the following scenarios: https://rubular.com/r/5xkN2wCqUr4mWN

  • "slug-with-dashes",
  • "slug/with/forward/slashes",
  • "slug.with.dots",
  • "slug-with-(brackets)",
  • "slug_with_underscore",
  • "slug*with*stars",
  • "slug1with2numbers3"
  • "slug,with,comma"
  • "slug%with%percentage"
  • "slug:with:colon"
  • "slug?with?question?mark"
  • "slug'with'quote"

Here is a ruby file that can be used for testing

@pepinho24 pepinho24 self-assigned this Nov 8, 2021
@pepinho24 pepinho24 added est: low Estimated for 1-2 dev days sev: high Impacts all documentations sites labels Nov 8, 2021
@pepinho24 pepinho24 linked a pull request Nov 8, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working est: low Estimated for 1-2 dev days sev: high Impacts all documentations sites
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants