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 <david.schulz@qt.io>

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<QString, QUrl> &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 !links().isEmpty();
-        return true;
-    if (QUrl(m_helpId).isValid())
-        return true;
-    return false;
 }
 
 QString HelpItem::extractContent(bool extended) const
@@ -87,14 +113,7 @@ QString HelpItem::extractContent(bool extended) const
         htmlExtractor.setMode(Utils::HtmlDocExtractor::FirstParagraph);
 
     QString contents;
-    QMap<QString, QUrl> helpLinks = links();
+    for (const QUrl &url : 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) {
         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<QString, QUrl> HelpItem::links() const
+const QMap<QString, QUrl> &HelpItem::links() const
 {
-    if (!m_helpLinks)
+    if (!m_helpLinks) {
-        m_helpLinks = Core::HelpManager::linksForIdentifier(m_helpId);
+        if (!m_helpUrl.isEmpty())
+            m_helpLinks.emplace(QMap<QString, QUrl>({{m_helpUrl.toString(), m_helpUrl}}));
+        else
+            m_helpLinks = Core::HelpManager::linksForIdentifier(m_helpId);
+    }
     return *m_helpLinks;
 }

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<QString, QUrl> &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<QString, QUrl> &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<QString, QUrl> links() const;
+    const QMap<QString, QUrl> &links() const;
 
 private:
+    QUrl m_helpUrl;
     QString m_helpId;
     QString m_docMark;
     Category m_category = Unknown;

src/plugins/help/helpplugin.cpp

@@ -657,12 +657,9 @@ void HelpPluginPrivate::requestContextHelp()
 
 void HelpPluginPrivate::showContextHelp(const HelpItem &contextHelp)
 {
-    QMap<QString, QUrl> links = contextHelp.links();
+    const QMap<QString, QUrl> &links = contextHelp.links();
-    // Maybe the id is already an URL
-    if (links.isEmpty() && LocalHelpManager::isValidUrl(contextHelp.helpId()))
-        links.insert(contextHelp.helpId(), contextHelp.helpId());
 
-    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),

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[] =

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);
 

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(),
+            setLastHelpItemIdentified(
-                                                     m_docFragment,
+                Core::HelpItem(url, m_docFragment, Core::HelpItem::QMakeVariableOfFunction));
-                                                     Core::HelpItem::QMakeVariableOfFunction));
         } else {
             // General qmake manual will be shown outside any function or variable
             setLastHelpItemIdentified("qmake");

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
+            // Use the URL, to disambiguate different versions
-            helpId = filteredUrlMap.first().toString();
+            const HelpItem helpItem(filteredUrlMap.first(),
-            const HelpItem helpItem(helpId, qName.join(QLatin1Char('.')), HelpItem::QmlComponent, filteredUrlMap);
+                                    qName.join(QLatin1Char('.')),
+                                    HelpItem::QmlComponent,
+                                    filteredUrlMap);
             setLastHelpItemIdentified(helpItem);
             return true;
         }