From d386b3c2412c4e0fb2dd32a51ebcf83ca2a32442 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Mon, 28 Jan 2019 12:46:30 +0100 Subject: [PATCH] Help: Avoid ambiguity of help ID being interpreted as URL Users know if they have a URL or not, we should not guess (and then even guess differently at different places) Change-Id: Iaaf69a94baadbee0ff427a2bc9065b714dcf8478 Reviewed-by: David Schulz --- src/plugins/coreplugin/helpitem.cpp | 57 +++++++++++++------ src/plugins/coreplugin/helpitem.h | 10 +++- src/plugins/help/helpplugin.cpp | 7 +-- src/plugins/help/localhelpmanager.cpp | 18 ------ src/plugins/help/localhelpmanager.h | 2 - .../profilehoverhandler.cpp | 5 +- src/plugins/qmljseditor/qmljshoverhandler.cpp | 8 ++- 7 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/plugins/coreplugin/helpitem.cpp b/src/plugins/coreplugin/helpitem.cpp index fd7bebb52f2..d2d444f44b4 100644 --- a/src/plugins/coreplugin/helpitem.cpp +++ b/src/plugins/coreplugin/helpitem.cpp @@ -40,6 +40,26 @@ HelpItem::HelpItem(const QString &helpId) : m_helpId(helpId) {} +HelpItem::HelpItem(const QUrl &url) + : m_helpUrl(url) +{} + +HelpItem::HelpItem(const QUrl &url, const QString &docMark, HelpItem::Category category) + : m_helpUrl(url) + , m_docMark(docMark) + , m_category(category) +{} + +HelpItem::HelpItem(const QUrl &url, + const QString &docMark, + HelpItem::Category category, + const QMap &helpLinks) + : m_helpUrl(url) + , m_docMark(docMark) + , m_category(category) + , m_helpLinks(helpLinks) +{} + HelpItem::HelpItem(const QString &helpId, const QString &docMark, Category category) : m_helpId(helpId), m_docMark(docMark), m_category(category) {} @@ -49,6 +69,16 @@ HelpItem::HelpItem(const QString &helpId, const QString &docMark, Category categ m_helpId(helpId), m_docMark(docMark), m_category(category), m_helpLinks(helpLinks) {} +void HelpItem::setHelpUrl(const QUrl &url) +{ + m_helpUrl = url; +} + +const QUrl &HelpItem::helpUrl() const +{ + return m_helpUrl; +} + void HelpItem::setHelpId(const QString &id) { m_helpId = id; } @@ -69,13 +99,9 @@ HelpItem::Category HelpItem::category() const bool HelpItem::isValid() const { - if (m_helpId.isEmpty()) + if (m_helpUrl.isEmpty() && m_helpId.isEmpty()) return false; - if (!links().isEmpty()) - return true; - if (QUrl(m_helpId).isValid()) - return true; - return false; + return !links().isEmpty(); } QString HelpItem::extractContent(bool extended) const @@ -87,14 +113,7 @@ QString HelpItem::extractContent(bool extended) const htmlExtractor.setMode(Utils::HtmlDocExtractor::FirstParagraph); QString contents; - QMap helpLinks = links(); - if (helpLinks.isEmpty()) { - // Maybe this is already an URL... - QUrl url(m_helpId); - if (url.isValid()) - helpLinks.insert(m_helpId, m_helpId); - } - foreach (const QUrl &url, helpLinks) { + for (const QUrl &url : links()) { const QString html = QString::fromUtf8(Core::HelpManager::fileData(url)); switch (m_category) { case Brief: @@ -135,9 +154,13 @@ QString HelpItem::extractContent(bool extended) const return contents; } -QMap HelpItem::links() const +const QMap &HelpItem::links() const { - if (!m_helpLinks) - m_helpLinks = Core::HelpManager::linksForIdentifier(m_helpId); + if (!m_helpLinks) { + if (!m_helpUrl.isEmpty()) + m_helpLinks.emplace(QMap({{m_helpUrl.toString(), m_helpUrl}})); + else + m_helpLinks = Core::HelpManager::linksForIdentifier(m_helpId); + } return *m_helpLinks; } diff --git a/src/plugins/coreplugin/helpitem.h b/src/plugins/coreplugin/helpitem.h index f06dcdc5c75..ef07bdbb069 100644 --- a/src/plugins/coreplugin/helpitem.h +++ b/src/plugins/coreplugin/helpitem.h @@ -57,6 +57,13 @@ public: HelpItem(const QString &helpId, const QString &docMark, Category category); HelpItem(const QString &helpId, const QString &docMark, Category category, const QMap &helpLinks); + HelpItem(const QUrl &url); + HelpItem(const QUrl &url, const QString &docMark, Category category); + HelpItem(const QUrl &url, const QString &docMark, Category category, + const QMap &helpLinks); + + void setHelpUrl(const QUrl &url); + const QUrl &helpUrl() const; void setHelpId(const QString &id); const QString &helpId() const; @@ -71,9 +78,10 @@ public: QString extractContent(bool extended) const; - QMap links() const; + const QMap &links() const; private: + QUrl m_helpUrl; QString m_helpId; QString m_docMark; Category m_category = Unknown; diff --git a/src/plugins/help/helpplugin.cpp b/src/plugins/help/helpplugin.cpp index ad090f58261..f0e253776b2 100644 --- a/src/plugins/help/helpplugin.cpp +++ b/src/plugins/help/helpplugin.cpp @@ -657,12 +657,9 @@ void HelpPluginPrivate::requestContextHelp() void HelpPluginPrivate::showContextHelp(const HelpItem &contextHelp) { - QMap links = contextHelp.links(); - // Maybe the id is already an URL - if (links.isEmpty() && LocalHelpManager::isValidUrl(contextHelp.helpId())) - links.insert(contextHelp.helpId(), contextHelp.helpId()); + const QMap &links = contextHelp.links(); - QUrl source = findBestLink(links); + const QUrl source = findBestLink(links); if (!source.isValid()) { // No link found or no context object HelpViewer *viewer = showHelpUrl(QUrl(Help::Constants::AboutBlank), diff --git a/src/plugins/help/localhelpmanager.cpp b/src/plugins/help/localhelpmanager.cpp index 731d938873b..3f698f2f284 100644 --- a/src/plugins/help/localhelpmanager.cpp +++ b/src/plugins/help/localhelpmanager.cpp @@ -302,24 +302,6 @@ BookmarkManager& LocalHelpManager::bookmarkManager() return *m_bookmarkManager; } -/*! - * Checks if the string does contain a scheme, and if that scheme is a "sensible" scheme for - * opening in a internal or external browser (qthelp, about, file, http, https). - * This is necessary to avoid trying to open e.g. "Foo::bar" in a external browser. - */ -bool LocalHelpManager::isValidUrl(const QString &link) -{ - QUrl url(link); - if (!url.isValid()) - return false; - const QString scheme = url.scheme(); - return (scheme == "qthelp" - || scheme == "about" - || scheme == "file" - || scheme == "http" - || scheme == "https"); -} - QByteArray LocalHelpManager::loadErrorMessage(const QUrl &url, const QString &errorString) { const char g_htmlPage[] = diff --git a/src/plugins/help/localhelpmanager.h b/src/plugins/help/localhelpmanager.h index fdb4f7874a6..bc8cd3b94a1 100644 --- a/src/plugins/help/localhelpmanager.h +++ b/src/plugins/help/localhelpmanager.h @@ -93,8 +93,6 @@ public: static QHelpEngine& helpEngine(); static BookmarkManager& bookmarkManager(); - static bool isValidUrl(const QString &link); - static QByteArray loadErrorMessage(const QUrl &url, const QString &errorString); Q_INVOKABLE static Help::Internal::LocalHelpManager::HelpData helpData(const QUrl &url); diff --git a/src/plugins/qmakeprojectmanager/profilehoverhandler.cpp b/src/plugins/qmakeprojectmanager/profilehoverhandler.cpp index f353d2ded33..6d24c13d837 100644 --- a/src/plugins/qmakeprojectmanager/profilehoverhandler.cpp +++ b/src/plugins/qmakeprojectmanager/profilehoverhandler.cpp @@ -63,9 +63,8 @@ void ProFileHoverHandler::identifyMatch(TextEditor::TextEditorWidget *editorWidg if (m_manualKind != UnknownManual) { QUrl url(QString::fromLatin1("qthelp://org.qt-project.qmake/qmake/qmake-%1-reference.html#%2") .arg(manualName()).arg(m_docFragment)); - setLastHelpItemIdentified(Core::HelpItem(url.toString(), - m_docFragment, - Core::HelpItem::QMakeVariableOfFunction)); + setLastHelpItemIdentified( + Core::HelpItem(url, m_docFragment, Core::HelpItem::QMakeVariableOfFunction)); } else { // General qmake manual will be shown outside any function or variable setLastHelpItemIdentified("qmake"); diff --git a/src/plugins/qmljseditor/qmljshoverhandler.cpp b/src/plugins/qmljseditor/qmljshoverhandler.cpp index 9421941deaa..fb6af001083 100644 --- a/src/plugins/qmljseditor/qmljshoverhandler.cpp +++ b/src/plugins/qmljseditor/qmljshoverhandler.cpp @@ -190,9 +190,11 @@ bool QmlJSHoverHandler::setQmlTypeHelp(const ScopeChain &scopeChain, const Docum filteredUrlMap.insert(x.key(), x.value()); } if (!filteredUrlMap.isEmpty()) { - // Use the url as helpId, to disambiguate different versions - helpId = filteredUrlMap.first().toString(); - const HelpItem helpItem(helpId, qName.join(QLatin1Char('.')), HelpItem::QmlComponent, filteredUrlMap); + // Use the URL, to disambiguate different versions + const HelpItem helpItem(filteredUrlMap.first(), + qName.join(QLatin1Char('.')), + HelpItem::QmlComponent, + filteredUrlMap); setLastHelpItemIdentified(helpItem); return true; }