fix: parse knowledge base article headings new line and tweak breadcrumbs#2505
Conversation
…umbs The KB article was not being split correctly by line so the entire article was being reproduced in the title tag, and this also resulted in repeated headings. At the same time, tweak the layout of the page to put the navigation at the end of the article, add the KMKBxxxx id to the title, and improve the breadcrumb navigation for KB articles. Fixes: #2503 Test-bot: skip
User Test ResultsTest specification and instructions User tests are not required |
| // KB articles are served by a single file, so we need to look for the | ||
| // global variable $kb_title to get the appropriate title, which is set | ||
| // in /knowledge-base/index.php. | ||
| $crumbs[2] = isset($kb_title) ? $kb_title : ''; |
There was a problem hiding this comment.
From devin.ai:
KB article title in breadcrumbs is mangled by URL-segment str_replace that converts dashes/underscores to spaces
The new code at _includes/includes/template.php:113 inserts the human-readable $kb_title (e.g. "FIX: Problems uninstalling Keyman 3.2 - 6.2 (KMKB0008)") into the $crumbs array, which is designed for URL path segments. This title then gets processed by the str_replace at line 117 that replaces - with spaces, _ with spaces, and removes .php — a transformation designed for URL slugs, not readable titles. This corrupts any KB title containing dashes (at least 15 articles are affected), e.g. "Keyman 3.2 - 6.2" becomes "Keyman 3.2 6.2". The same mangling also occurs at line 130 when computing $crumb2.
There was a problem hiding this comment.
This is a longstanding limitation with the breadcrumbs. A proper fix would be to read the title from each page, but that's more work. I think we leave it as is for now; it's only the breadcrumbs.
| } | ||
| } | ||
| } | ||
| echo "</ul>"; |
There was a problem hiding this comment.
From devin.ai:
Stray tag emitted when viewing a KB article (pre-existing)
At knowledge-base/index.php:88, echo "</ul>"; is always executed regardless of which branch of the if($id) / else block runs. When viewing a specific article ($id is truthy), no <ul> was opened, so this produces a stray </ul> in the HTML output. This is a pre-existing issue (the old code had the same structure at old line 85) and was not introduced by this PR, but it persists in the restructured code.
| // We test the first line of the file for a title. | ||
|
|
||
| $text = explode('\n', $kb); | ||
| $text = explode("\n", $kb); |
There was a problem hiding this comment.
Took me a while (and devin.ai to point it out) to realize that this is what fixes the bug 😄.
3 participants
The KB article was not being split correctly by line so the entire article was being reproduced in the title tag, and this also resulted in repeated headings.
At the same time, tweak the layout of the page to put the navigation at the end of the article, add the KMKBxxxx id to the title, and improve the breadcrumb navigation for KB articles.
Header for article (breadcrumbs, title)
Footer for article (navigation)
Fixes: #2503
Test-bot: skip