From 239827baad0301d578a861be55d5711d82cc2048 Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Sun, 22 Nov 2020 01:00:36 +0100 Subject: [PATCH] Rework how we open urls that have a # Previously if it was a remote url that had # and a . after the # we assumed the url had no fragment and everything was filename. We don't do that anymore, what we do now is try to open the url as parsed, i.e. before the # is the filename after is the fragment, and if that fails we try to open everything as filename and nothing as fragment. Unfortunately given how kpart internals handle opening local vs remote urls we need to do this in two places. Also we have to remove the test that checked that the url was mangled at the shell level because we don't do that anymore. Unfortunately can't add a test for the new codepage since it would involve starting an http server ^_^ Filenames: source2e.pdf foo#bar.pdf What works: * okular http://localhost/source2e.pdf#subsection.68.3 * okular file:///srv/http/source2e.pdf#subsection.68.3 * okular source2e.pdf#subsection.68.3 (in the /srv/http folder) * okular source2e.pdf#2 * okular http://localhost/foo#bar.pdf * okular file:///srv/http/foo#bar.pdf * okular foo#bar.pdf (in the /srv/http folder) What doesn't work: * okular http://localhost/foo#bar.pdf#2 I think it's a fair limitation that if you want to open a file that contains # in the name and also use a # page marker you need to use the encoded url like okular http://localhost/foo%23bar.pdf#2 after all things like firefox will totally fail opening http://localhost/foo#bar.pdf and will just work if you give the encoded url BUGS: 426976 --- autotests/shelltest.cpp | 6 ------ part/part.cpp | 27 ++++++++++++++++++++++----- part/part.h | 6 ++++++ shell/shellutils.cpp | 7 ------- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/autotests/shelltest.cpp b/autotests/shelltest.cpp index 9d097bbe8..43aceb607 100644 --- a/autotests/shelltest.cpp +++ b/autotests/shelltest.cpp @@ -64,12 +64,6 @@ void ShellTest::testUrlArgs_data() // non-local files QTest::newRow("http://kde.org/foo.pdf") << "http://kde.org/foo.pdf" << true << QUrl(QStringLiteral("http://kde.org/foo.pdf")); - // make sure we don't have a fragment - QUrl hashInName(QStringLiteral("http://kde.org")); - QVERIFY(hashInName.path().isEmpty()); - hashInName.setPath(QStringLiteral("/foo#bar.pdf")); - QVERIFY(hashInName.fragment().isEmpty()); - QTest::newRow("http://kde.org/foo#bar.pdf") << "http://kde.org/foo#bar.pdf" << true << hashInName; QUrl withAnchor(QStringLiteral("http://kde.org/foo.pdf")); withAnchor.setFragment(QStringLiteral("anchor")); QTest::newRow("http://kde.org/foo.pdf#anchor") << "http://kde.org/foo.pdf#anchor" << true << withAnchor; diff --git a/part/part.cpp b/part/part.cpp index 0f1da3ff4..536e412a4 100644 --- a/part/part.cpp +++ b/part/part.cpp @@ -1152,7 +1152,9 @@ void Part::loadCancelled(const QString &reason) // so we don't want to show an ugly messagebox just because the document is // taking more than usual to be recreated if (m_viewportDirty.pageNumber == -1) { - if (!reason.isEmpty()) { + if (m_urlWithFragment.isValid() && !m_urlWithFragment.isLocalFile()) { + tryOpeningUrlWithFragmentAsName(); + } else if (!reason.isEmpty()) { KMessageBox::error(widget(), i18n("Could not open %1. Reason: %2", url().toDisplayString(), reason)); } } @@ -1685,6 +1687,7 @@ bool Part::openUrl(const QUrl &_url, bool swapInsteadOfOpening) QUrl url(_url); if (url.hasFragment()) { + m_urlWithFragment = _url; const QString dest = url.fragment(QUrl::FullyDecoded); bool ok = true; int page = dest.toInt(&ok); @@ -1709,6 +1712,8 @@ bool Part::openUrl(const QUrl &_url, bool swapInsteadOfOpening) m_document->setNextDocumentDestination(dest); } url.setFragment(QString()); + } else { + m_urlWithFragment.clear(); } // this calls in sequence the 'closeUrl' and 'openFile' methods @@ -1719,15 +1724,27 @@ bool Part::openUrl(const QUrl &_url, bool swapInsteadOfOpening) setWindowTitleFromDocument(); } else { - resetStartArguments(); - /* TRANSLATORS: Adding the reason (%2) why the opening failed (if any). */ - QString errorMessage = i18n("Could not open %1. %2", url.toDisplayString(), QStringLiteral("\n%1").arg(m_document->openError())); - KMessageBox::error(widget(), errorMessage); + if (m_urlWithFragment.isValid() && m_urlWithFragment.isLocalFile()) { + openOk = tryOpeningUrlWithFragmentAsName(); + } else { + resetStartArguments(); + /* TRANSLATORS: Adding the reason (%2) why the opening failed (if any). */ + QString errorMessage = i18n("Could not open %1. %2", url.toDisplayString(), QStringLiteral("\n%1").arg(m_document->openError())); + KMessageBox::error(widget(), errorMessage); + } } return openOk; } +bool Part::tryOpeningUrlWithFragmentAsName() +{ + QUrl url = m_urlWithFragment; + url.setPath(url.path() + QLatin1Char('#') + url.fragment()); + url.setFragment(QString()); + return openUrl(url); +} + bool Part::queryClose() { if (!isReadWrite() || !isModified()) diff --git a/part/part.h b/part/part.h index 8b3b474cd..ce9fd00de 100644 --- a/part/part.h +++ b/part/part.h @@ -296,6 +296,8 @@ private: void slotShareActionFinished(const QJsonObject &output, int error, const QString &message); #endif + bool tryOpeningUrlWithFragmentAsName(); + static int numberOfParts; QTemporaryFile *m_tempfile; @@ -421,6 +423,10 @@ private: // String to search in document startup QString m_textToFindOnOpen; + // Set when opening an url that had fragment so that if it fails opening we try adding the fragment to the filename + // if we're opening http://localhost/foo#bar.pdf and the filename contains an # we can open it after trying to open foo fails + QUrl m_urlWithFragment; + private Q_SLOTS: void slotAnnotationPreferences(); void slotHandleActivatedSourceReference(const QString &absFileName, int line, int col, bool *handled); diff --git a/shell/shellutils.cpp b/shell/shellutils.cpp index 69189acca..f49d072fd 100644 --- a/shell/shellutils.cpp +++ b/shell/shellutils.cpp @@ -45,13 +45,6 @@ QUrl urlFromArg(const QString &_arg, FileExistFunc exist_func, const QString &pa url.setPath(path.left(hashIndex)); url.setFragment(path.mid(hashIndex + 1)); } - } else if (!url.fragment().isEmpty()) { - // make sure something like http://example.org/foo#bar.pdf is treated as a path name - // but something like http://example.org/foo.pdf#bar is foo.pdf plus an anchor "bar" - if (url.fragment().contains(QLatin1Char('.'))) { - url.setPath(url.path() + QLatin1Char('#') + url.fragment()); - url.setFragment(QString()); - } } if (!pageArg.isEmpty()) { url.setFragment(pageArg);