Differential D16057

Fix opening URL's to help files
ClosedPublic
Actions

Authored by pavelmos on Oct 9 2018, 6:49 AM.

Details

Reviewers
yurchor
ltoscano
Commits
R125:d90b712b0610: Fix opening URL's to help files
Summary

Signed-off-by: Pavel Mars <mars@basealt.ru>

When you select the menu item "KInfoCenter Modules" -> "DMA-Channels" (or another item: Memory, OpenGL, etc.) -> "KInfoCenter" -> "Start KInfoCenter", the help on the right side of the program screen is incorrect.

When you select any of the submenus that appear, the same help is displayed (../index.html).
This is due to the incorrect formation of links to help.
To fix this bug, add "QUrl :: RemoveFragment" to the string "mToc-> setApplication (url.toString (QUrl :: RemoveScheme | QUrl :: RemoveFilename | QUrl :: StripTrailingSlash))" in the navigatorappitem.cpp file.
This expression removes the fragment from the generated link and allows you to properly load the help file.

Diff Detail

Repository
R125 KHelpCenter
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
pavelmos created this revision.Oct 9 2018, 6:49 AM
Restricted Application added a project: Documentation. · View Herald TranscriptOct 9 2018, 6:49 AM
Restricted Application added a subscriber: kde-doc-english. · View Herald Transcript
pavelmos requested review of this revision.Oct 9 2018, 6:49 AM
pavelmos edited the summary of this revision. (Show Details)Oct 9 2018, 6:51 AM
pavelmos edited the test plan for this revision. (Show Details)
pavelmos added reviewers: yurchor, ltoscano.
yurchor accepted this revision.Oct 9 2018, 6:54 AM
This revision is now accepted and ready to land.Oct 9 2018, 6:54 AM
pavelmos added a comment.Oct 19 2018, 9:24 AM

Do I need to do anything else with this patch?
When will it be included in the KHelpCenter?

ltoscano requested changes to this revision.Oct 19 2018, 9:26 AM

Uh, I lost track of the patches removed and readded.

Please follow the same rule for the commit message.

Also, I did not explain it correctly: the line about the bug is not
BUG: <num>

but

BUG: num

This revision now requires changes to proceed.Oct 19 2018, 9:26 AM
pavelmos edited the summary of this revision. (Show Details)Oct 19 2018, 9:42 AM
pavelmos added a comment.Oct 19 2018, 11:37 AM

I fixed everything

ltoscano added a comment.Oct 21 2018, 1:42 PM

Not the commit message; for example, "STEPS TO REPRODUCE BUG" do not belong here.

Another user proposed a different fix for this issue:
https://phabricator.kde.org/D16352

I'm still not sure which is the better fix, but for sure you can take a look at the other commit message as an example on how to write a commit message.

pavelmos added a comment.Oct 22 2018, 7:31 AM
In D16057#346697, @ltoscano wrote:

Not the commit message; for example, "STEPS TO REPRODUCE BUG" do not belong here.

Another user proposed a different fix for this issue:
https://phabricator.kde.org/D16352

I'm still not sure which is the better fix, but for sure you can take a look at the other commit message as an example on how to write a commit message.

These are two different issues!
"Contents items expanding issue" - is not "Fix sublist expanding on the left panel".

pavelmos added a comment.Oct 22 2018, 8:33 AM
In D16057#346697, @ltoscano wrote:

Not the commit message; for example, "STEPS TO REPRODUCE BUG" do not belong here.

Another user proposed a different fix for this issue:
https://phabricator.kde.org/D16352

I'm still not sure which is the better fix, but for sure you can take a look at the other commit message as an example on how to write a commit message.

I'll change the description now

pavelmos edited the summary of this revision. (Show Details)Oct 22 2018, 1:08 PM
pavelmos edited the test plan for this revision. (Show Details)
pavelmos edited the summary of this revision. (Show Details)Oct 23 2018, 7:20 AM
pavelmos retitled this revision from Fix sublist expanding on the left panel to Fix opening URL's to help files.Oct 23 2018, 7:24 AM
pavelmos edited the summary of this revision. (Show Details)Oct 31 2018, 7:51 AM

I corrected the description. Do you have any comments?

ltoscano added a comment.Nov 19 2018, 12:25 AM

After fixing https://bugs.kde.org/show_bug.cgi?id=399274, the duplicated menu under KInfoCenter Modules" -> <module_name> -> "KInfoCenter" does not exist anymore. Is this fix still relevant then?

i.Dark_Templar added a subscriber: i.Dark_Templar.Nov 19 2018, 8:25 PM

After fixing bug 399274, the issue this change fixes becomes masked, but it still remains. It's hard to hit the issue now, maybe even impossible at the moment. But it might reappear later.

For example, if removal of duplicate submenus is reworked into an option (and disabled) or removed, this issue would reappear. Or if there would appear a menu item with a fragment set in contents tree (url://example#fragment), or multiple such items with same base url and different fragment (url://example#fragment2, url://example#fragment3, etc), and at the same time no menu item having corresponding url with empty fragment would exist in that tree (no "url://example"), in that unlikely case menu items deduplication wouldn't work and this issue would reappear too.

Those are two theoretic scenarios I can imagine at the moment which would be fixed by this change.

pavelmos added a comment.Apr 3 2019, 12:29 PM
In D16057#361900, @ltoscano wrote:

After fixing https://bugs.kde.org/show_bug.cgi?id=399274, the duplicated menu under KInfoCenter Modules" -> <module_name> -> "KInfoCenter" does not exist anymore. Is this fix still relevant then?

i.Dark_Templar is right. Did he answer your question?

ltoscano accepted this revision.Apr 4 2019, 10:50 PM

Yes, sorry, I apologize.

This revision is now accepted and ready to land.Apr 4 2019, 10:50 PM
Closed by commit R125:d90b712b0610: Fix opening URL's to help files (authored by pavelmos, committed by ltoscano). · Explain WhyApr 4 2019, 10:51 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mnavigatorappitem.cpp (2 lines)

Diff 55445

View Options

navigatorappitem.cpp

Loading...