If KHelpCenter isn't available fallback to opening doc at docs.kde.org
ClosedPublic

Authored by ahmadsamir on Feb 20 2020, 1:06 PM.

Details

Summary

UrlHandler from KGUIAddons sets a handler for help:/ urls, which opens
khelpcenter if it's available or falls back to opening the relevant page
at docs.kde.org.

Explicitly add KGuiAddons to find_package() and target_link_libraries().

Depends on D27615

BUG: 405647

FIXED-IN: 5.18.1

Test Plan
  • make sure khelpcenter is not installed and open systemsettings
  • navigate to any module (one that has Help visible, e.g. Fonts), "Help" button is disabled
  • install khelpcenter and restart systemsettings, the "Help" button should be enabled

Diff Detail

Repository
R124 System Settings
Branch
l-khelp (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22843
Build 22861: arc lint + arc unit
ahmadsamir created this revision.Feb 20 2020, 1:06 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 20 2020, 1:06 PM
ahmadsamir requested review of this revision.Feb 20 2020, 1:06 PM
apol requested changes to this revision.Feb 20 2020, 2:39 PM
apol added a subscriber: apol.

If khelpcenter is not available, we do an openUrl that should show the documentation on your browser.

This revision now requires changes to proceed.Feb 20 2020, 2:39 PM

You mean open the local .docbook with kio_help to get an HTML then open it in a browser? or opening docs.kde.org e.g. https://docs.kde.org/stable5/en/kde-workspace/kcontrol/fonts/index.html ?

apol added a comment.Feb 21 2020, 3:11 PM

You mean open the local .docbook with kio_help to get an HTML then open it in a browser? or opening docs.kde.org e.g. https://docs.kde.org/stable5/en/kde-workspace/kcontrol/fonts/index.html ?

This is what I mean:
https://lxr.kde.org/source/frameworks/kguiaddons/src/util/urlhandler.cpp#0033

openHelp() will need to be changed, because the "application=" part of the url will be "systemsettings" (QCoreApplication::applicationName()), which would open the handbook of systemsettings instead of e.g. ..../kcontrol/fonts/index.html.

Is the API for the redirects in the docs.kde.org url documented somewhere?

ahmadsamir updated this revision to Diff 76259.Feb 24 2020, 8:12 AM
ahmadsamir retitled this revision from Only enable the "Help" button if khelpcenter is available to If KHelpCenter isn't available fallback to opening doc at docs.kde.org.
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir removed a subscriber: apol.

Use UrlHandler from KGuiAddons

ahmadsamir updated this revision to Diff 76265.Feb 24 2020, 8:27 AM
ahmadsamir edited the summary of this revision. (Show Details)

Change BUG:

I wonder how to make systemsettings depend on KGuiAddons (if that's needed here)?

ahmadsamir updated this revision to Diff 76266.Feb 24 2020, 8:35 AM
ahmadsamir edited the summary of this revision. (Show Details)

find_package() KGuiAddons

apol added a comment.Feb 24 2020, 4:06 PM

Makes sense. Have you tested that this works? You should probably pass KF5::GuiAddons to the target_link_libraries as well.

ahmadsamir edited the summary of this revision. (Show Details)

target_link_libraries() KGuiAddons

In D27518#617038, @apol wrote:

Makes sense. Have you tested that this works? You should probably pass KF5::GuiAddons to the target_link_libraries as well.

I tested a good number of the KCMs, and it seems to work well (both when khelpcenter is available and when it is not).

apol added a comment.Feb 25 2020, 5:54 PM
In D27518#617038, @apol wrote:

Makes sense. Have you tested that this works? You should probably pass KF5::GuiAddons to the target_link_libraries as well.

I tested a good number of the KCMs, and it seems to work well (both when khelpcenter is available and when it is not).

Please link against KF5::GuiAddons from the target that needs to support help:

target_link_libraries() KGuiAddons in the sysetmsettings5 target

apol accepted this revision.Feb 26 2020, 1:47 PM
This revision is now accepted and ready to land.Feb 26 2020, 1:47 PM
This revision was automatically updated to reflect the committed changes.
sitter added a subscriber: sitter.Apr 7 2020, 10:57 AM

FYI linking a library without actually using any symbols is not going to do anything in practice because many distros link with --as-needed which undoes unnecessary linking. also see https://markmail.org/message/m3mdrsfgxoyfjtte

FYI linking a library without actually using any symbols is not going to do anything in practice because many distros link with --as-needed which undoes unnecessary linking. also see https://markmail.org/message/m3mdrsfgxoyfjtte

Yes, some distros do that indeed (the ones I am sure of are Mandriva, Mageia, OpenSuse); IINM, a, hacky, way to find out when compiling is to omit the entry from target_link_libraries(), if the compilation succeeds, then it's not NEEDED, and we needn't add it.

My point is that this diff adds kguiaddons as a link library but doesn't actually use any symbol, so it may as well not be a link library (or something ought to call some function of kguiaddons so it actually links it) ;)

My point is that this diff adds kguiaddons as a link library but doesn't actually use any symbol, so it may as well not be a link library (or something ought to call some function of kguiaddons so it actually links it) ;)

I got/agree with the first half of your point, "so it may as well not be a link library".

The thing is systemsettings5 will always pull kguiaddons as an indirect dependency:

  • systemsettings5 requires libKF5ConfigWidgets.so.5
  • libKF5ConfigWidgets5 requires libKF5GuiAddons.so.5

I am talking about rpm-based distros (since rpm is the package-system I am familiar with), I am sure it's the same in other distros/package managers.