From d626d3ba75df32031a739a15eca49554ee0073e5 Mon Sep 17 00:00:00 2001 From: Jan Wester Date: Fri, 6 Sep 2019 07:55:18 +0200 Subject: [PATCH] Fixed & robustified image tooltips (issue #593) Last time I touched this code, I went in looking for a specific problem, and came out with a fix specific to that issue. That fix was not wrong, yet it hardly covered all the problems present in the code once one took into account issues like: - local vs remote resources, - relative vs absolute paths, - different operating systems behaving differently, and - Qt being uniquely buggy on different platforms. The major part of it was fixed by using QUrl.fromUserInput(), which does the exact kind of auto-detection for the nature of the resource that we were in need of. The rest of the issues were fixed by creating a number of test cases and fixing problems as they popped up. Testing was done in Windows & Ubunty against the above-mentioned test cases, which can be found in PR #629. Regarding ImageTooltip.supportedSchemes When QUrl.fromUserInput() misidentifies the scheme on Linux, it causes all resemblance between the original request and the reply.request() in the finished() signal to be lost, which results in this item getting stuck in the ImageTooltip processing pipeline. Limiting the supported schemes to the ones most commonly encountered ('file', 'http', 'https' and the schema-less local paths) is the only reliable method I have found to work around this particular bug in Qt. --- manuskript/ui/views/MDEditView.py | 52 ++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/manuskript/ui/views/MDEditView.py b/manuskript/ui/views/MDEditView.py index 2c47b409..eb261e13 100644 --- a/manuskript/ui/views/MDEditView.py +++ b/manuskript/ui/views/MDEditView.py @@ -591,6 +591,8 @@ class ImageTooltip: manager = QNetworkAccessManager() processing = {} + supportedSchemes = ("", "file", "http", "https") + def fromUrl(url, pos, editor): """ Shows the image tooltip for the given url if available, or requests it for future use. @@ -605,23 +607,64 @@ class ImageTooltip: except: pass # already connected - qurl = QUrl(url) - if (qurl in ImageTooltip.processing): + qurl = QUrl.fromUserInput(url) + if (qurl == QUrl()): + ImageTooltip.cache[url] = (False, ImageTooltip.manager.tr("The image path or URL is incomplete or malformed.")) + ImageTooltip.showTooltip(url, pos) + return # empty QUrl means it failed completely + elif (qurl.scheme() not in ImageTooltip.supportedSchemes): + # QUrl.fromUserInput() can occasionally deduce an incorrect scheme, + # which produces an error message regarding an unknown scheme. (Yay!) + # But it also breaks all possible methods to try and associate the + # reply with the original request in finished(), since reply.request() + # is completely and utterly butchered for all tracking needs. :'( + # (The QNetworkRequest, .url() and .originatingObject() can all change.) + + # Test case (Linux): ![image](C:\test_root.jpg) + ImageTooltip.cache[url] = (False, ImageTooltip.manager.tr("The protocol \"{}\" is not supported.").format(qurl.scheme())) + ImageTooltip.showTooltip(url, pos) + return # no more request/reply chaos, please! + elif (qurl in ImageTooltip.processing): return # one download is more than enough # Request the image for later processing. request = QNetworkRequest(qurl) ImageTooltip.processing[qurl] = (pos, url) - ImageTooltip.manager.get(request) + reply = ImageTooltip.manager.get(request) + + # On Linux the finished() signal is not triggered when the url resembles + # 'file://X:/...'. But because it completes instantly, we can manually + # trigger the code to keep our processing dictionary neat & clean. + if reply.error() == 302: # QNetworkReply.ProtocolInvalidOperationError + ImageTooltip.finished(reply) def finished(reply): """ After retrieving an image, we add it to the cache. """ cache = ImageTooltip.cache + url_key = reply.request().url() + pos, url = None, None + + if url_key in ImageTooltip.processing: + # Obtain the information associated with this request. + pos, url = ImageTooltip.processing[url_key] + del ImageTooltip.processing[url_key] + elif len(ImageTooltip.processing) == 0: + # We are not processing anything. Maybe it is a spurious signal, + # or maybe the 'reply.error() == 302' workaround in fromUrl() has + # been fixed in Qt. Whatever the reason, we can assume this request + # has already been handled, and needs no more work from us. + return + else: + # Somehow we lost track. Log what we can to hopefully figure it out. + print("Warning: unable to match fetched data for tooltip to original request.") + print("- Completed request:", url_key) + print("- Status upon finishing:", reply.error(), reply.errorString()) + print("- Currently processing:", ImageTooltip.processing) + return # Update cache with retrieved data. - pos, url = ImageTooltip.processing[reply.request().url()] if reply.error() != QNetworkReply.NoError: cache[url] = (False, reply.errorString()) else: @@ -629,7 +672,6 @@ class ImageTooltip: px.loadFromData(reply.readAll()) px = px.scaled(800, 600, Qt.KeepAspectRatio) cache[url] = (True, px) - del ImageTooltip.processing[reply.request().url()] ImageTooltip.showTooltip(url, pos)