Merge pull request #5528 from opengisch/huh

Improve robustness of our handling of file path to source URLs

.github/workflows/linux.yml

@@ -47,7 +47,7 @@ jobs:
       - name: 🔨 Prepare build env
         run: |
           sudo apt-get update
-          sudo apt-get install -y gperf autopoint '^libxcb.*-dev' libx11-xcb-dev libegl1-mesa libegl1-mesa-dev libgl1-mesa-dev libglu1-mesa-dev mesa-common-dev libxrender-dev libxi-dev libxkbcommon-dev libxkbcommon-x11-dev libxrandr-dev libxxf86vm-dev autoconf-archive libgstreamer-gl1.0-0 libgstreamer-plugins-base1.0-0 libfuse2 libpulse-dev libcups2-dev nasm
+          sudo apt-get install -y gperf autopoint '^libxcb.*-dev' libx11-xcb-dev libegl1-mesa libegl1-mesa-dev libgl1-mesa-dev libglu1-mesa-dev mesa-common-dev libxrender-dev libxi-dev libxkbcommon-dev libxkbcommon-x11-dev libxrandr-dev libxxf86vm-dev autoconf-archive libgstreamer-gl1.0-0 libgstreamer-plugins-base1.0-0 libfuse2 libpulse-dev libcups2-dev nasm python3-tk
           # Required to run unit tests on linux
 
       - name: Install linuxdeploy

src/core/platforms/platformutilities.cpp

@@ -26,6 +26,7 @@
 #include "qgsmessagelog.h"
 #include "resourcesource.h"
 #include "stringutils.h"
+#include "urlutils.h"
 
 #include <QApplication>
 #include <QClipboard>
@@ -324,7 +325,7 @@ ResourceSource *PlatformUtilities::getFile( const QString &prefix, const QString
 
 ViewStatus *PlatformUtilities::open( const QString &uri, bool, QObject * )
 {
-  QDesktopServices::openUrl( QStringLiteral( "file://%1" ).arg( uri ) );
+  QDesktopServices::openUrl( UrlUtils::fromString( uri ) );
   return nullptr;
 }
 

src/core/utils/urlutils.cpp

@@ -17,9 +17,9 @@
 
 #include "urlutils.h"
 
+#include <QFileInfo>
 #include <QUrl>
 
-
 UrlUtils::UrlUtils( QObject *parent )
   : QObject( parent )
 {
@@ -28,8 +28,18 @@ UrlUtils::UrlUtils( QObject *parent )
 
 bool UrlUtils::isRelativeOrFileUrl( const QString &url )
 {
-  if ( url.startsWith( QStringLiteral( "file:///" ) ) )
+  if ( url.startsWith( QStringLiteral( "file://" ) ) )
     return true;
 
   return QUrl( url ).isRelative();
 }
+
+QUrl UrlUtils::fromString( const QString &string )
+{
+  if ( QFileInfo::exists( string ) )
+  {
+    return QUrl::fromLocalFile( string );
+  }
+
+  return QUrl( string );
+}

src/core/utils/urlutils.h

@@ -30,11 +30,13 @@ class QFIELD_CORE_EXPORT UrlUtils : public QObject
   public:
     explicit UrlUtils( QObject *parent = nullptr );
 
-
     /**
-     * Checks whether the provided string is a relative url (has no protocol or starts with `file://`).
+     * Checks whether the provided string is a relative \a url (has no protocol or starts with `file://`).
      */
     static Q_INVOKABLE bool isRelativeOrFileUrl( const QString &url );
+
+    //! Returns a URL from a \a string with logic to handle local paths
+    static Q_INVOKABLE QUrl fromString( const QString &string );
 };
 
 #endif // URLUTILS_H

src/qml/About.qml

@@ -136,7 +136,7 @@ Item {
                     label = dataDirs.length > 1 ? qsTr('QField app directories') : qsTr('QField app directory');
                     for (let dataDir of dataDirs) {
                         if (isDesktopPlatform) {
-                            label += '<br><a href="file://' + dataDir + '">' + dataDir + '</a>';
+                            label += '<br><a href="' + UrlUtils.fromString(dataDir) + '">' + dataDir + '</a>';
                         } else {
                             label += '<br>' + dataDir;
                         }

src/qml/FeatureListForm.qml

@@ -58,7 +58,7 @@ Rectangle {
   width: {
       if ( props.isVisible || featureForm.canvasOperationRequested )
       {
-          if (fullScreenView || parent.width < parent.height || parent.width < 300)
+          if (fullScreenView || parent.width <= parent.height || parent.width < 300)
           {
               parent.width
           }

src/qml/PluginManagerSettings.qml

@@ -86,7 +86,7 @@ Popup {
                 Layout.preferredWidth: 24
                 Layout.preferredHeight: 24
 
-                source: Icon !== '' ? 'file://' + Icon : ''
+                source: Icon !== '' ? UrlUtils.fromString(Icon) : ''
                 fillMode: Image.PreserveAspectFit
                 mipmap: true
               }

src/qml/QFieldAudioRecorder.qml

@@ -64,11 +64,7 @@ Popup {
     running: false
 
     onTriggered: {
-      var path = recorder.actualLocation.toString()
-      // On Android, the file protocol prefix is present while on Linux it isn't
-      var filePos = path.indexOf('file://')
-      path = filePos == -1 ? 'file://' + path : path
-      player.source = path
+      player.source = UrlUtils.fromString(recorder.actualLocation.toString())
     }
   }
 

src/qml/QFieldCamera.qml

@@ -127,7 +127,7 @@ Popup {
 
         onImageSaved: (requestId, path) => {
           currentPath  = path
-          photoPreview.source = 'file://'+path
+          photoPreview.source = UrlUtils.fromString(path)
           cameraItem.state = "PhotoPreview"
         }
       }

src/qml/QFieldSketcher.qml

@@ -220,7 +220,7 @@ Popup {
               anchors.margins: 5
               visible: templatePath !== ''
               fillMode: Image.PreserveAspectFit
-              source: templatePath !== '' ? 'file://' + templatePath : ''
+              source: templatePath !== '' ? UrlUtils.fromString(templatePath) : ''
             }
 
             Rectangle {

src/qml/editorwidgets/ExternalResource.qml

@@ -92,11 +92,16 @@ EditorWidgetBase {
         image.hasImage = true
         image.opacity = 1
         image.anchors.topMargin = 0
-        image.source = (!isHttp ? 'file://' : '') + fullValue
+        image.source = UrlUtils.fromString(fullValue)
         geoTagBadge.hasGeoTag = ExifTools.hasGeoTag(fullValue)
       } else if (isAudio || isVideo) {
+        mediaFrame.height = 48
+
+        image.visible = false
+        image.opacity = 0.5
+        image.source = ''
         player.firstFrameDrawn = false
-        player.sourceUrl = (!isHttp ? 'file://' : '') + fullValue
+        player.sourceUrl = UrlUtils.fromString(fullValue)
       }
     } else {
       image.source = ''
@@ -262,7 +267,7 @@ EditorWidgetBase {
       id: player
       active: isAudio || isVideo
 
-      property string sourceUrl: ''
+      property url sourceUrl: ''
       property bool firstFrameDrawn: false
 
       anchors.left: parent.left
@@ -324,7 +329,7 @@ EditorWidgetBase {
       id: sketchButton
       anchors.top: image.top
       anchors.right: image.right
-      visible: image.status === Image.Ready && isEnabled
+      visible: image.source != '' && image.status === Image.Ready && isEnabled
 
       round: true
       iconSource: Theme.getThemeVectorIcon( "ic_freehand_white_24dp" )

src/qml/editorwidgets/relationeditors/ordered_relation_editor.qml

@@ -246,7 +246,7 @@ EditorWidgetBase {
           Image {
             id: featureImage
             source: ImagePath
-                      ? ('file://' + ImagePath)
+                      ? UrlUtils.fromString(ImagePath)
                       : Theme.getThemeIcon("ic_photo_notavailable_black_24dp")
             width: parent.height
             height: parent.height

test/spix/requirements.txt

@@ -2,3 +2,5 @@ py
 pytest
 pytest-html
 pytest-image-diff
+pyautogui
+tk

test/spix/smoke_test.py

@@ -12,6 +12,7 @@
 import platform
 from pathlib import Path
 from PIL import Image
+import pyautogui
 
 
 @pytest.fixture
@@ -124,7 +125,7 @@ def test_wms_layer(app, screenshot_path, screenshot_check, extra, process_alive)
     messagesCount = 0
     for i in range(0, 10):
         message = app.getStringProperty(
-            "mainWindow/messageLog/messageItem_{}/messageText".format(i), "text"
+            f"mainWindow/messageLog/messageItem_{i}/messageText", "text"
         )
         if message == "":
             break
@@ -156,7 +157,7 @@ def test_projection(app, screenshot_path, screenshot_check, extra, process_alive
     messagesCount = 0
     for i in range(0, 10):
         message = app.getStringProperty(
-            "mainWindow/messageLog/messageItem_{}/messageText".format(i), "text"
+            f"mainWindow/messageLog/messageItem_{i}/messageText", "text"
         )
         if message == "":
             break
@@ -166,6 +167,52 @@ def test_projection(app, screenshot_path, screenshot_check, extra, process_alive
     assert messagesCount == 0
 
 
+@pytest.mark.project_file("test_image_attachment.qgz")
+def test_projection(app, screenshot_path, screenshot_check, extra, process_alive):
+    """
+    Starts a test app and check for proper reprojection support (including rendering check and message logs).
+    This also tests that QField is able to reach proj's crucial proj.db
+    """
+    assert app.existsAndVisible("mainWindow")
+
+    # Arbitrary wait period to insure project fully loaded and rendered
+    time.sleep(4)
+
+    messagesCount = 0
+    for i in range(0, 10):
+        message = app.getStringProperty(
+            f"mainWindow/messageLog/messageItem_{i}/messageText", "text"
+        )
+        if message == "":
+            break
+        extra.append(extras.html("Message logs content: {}".format(message)))
+        messagesCount = messagesCount + 1
+    extra.append(extras.html("Message logs count: {}".format(messagesCount)))
+    assert messagesCount == 0
+
+    bounds = app.getBoundingBox("mainWindow/mapCanvas")
+    move_x = bounds[0] + bounds[2] / 2
+    move_y = bounds[1] + bounds[3] / 3
+
+    pyautogui.moveTo(move_x, move_y, duration=0.5)
+    pyautogui.click(interval=0.5)
+
+    bounds = app.getBoundingBox("mainWindow/featureForm")
+    move_x = bounds[0] + bounds[2] / 2
+    move_y = bounds[1] + 80
+
+    pyautogui.moveTo(move_x, move_y, duration=0.5)
+    pyautogui.click(interval=0.5)
+
+    app.takeScreenshot(
+        "mainWindow", os.path.join(screenshot_path, "test_image_attachment.png")
+    )
+    assert process_alive()
+    extra.append(extras.html('<img src="images/test_image_attachment.png"/>'))
+
+    assert screenshot_check("test_image_attachment", "test_image_attachment")
+
+
 @pytest.mark.project_file("test_svg.qgz")
 def test_svg(app, screenshot_path, screenshot_check, extra, process_alive):
     """
@@ -198,7 +245,7 @@ def test_postgis_ssl(app, screenshot_path, screenshot_check, extra, process_aliv
     messagesCount = 0
     for i in range(0, 10):
         message = app.getStringProperty(
-            "mainWindow/messageLog/messageItem_{}/messageText".format(i), "text"
+            f"mainWindow/messageLog/messageItem_{i}/messageText", "text"
         )
         if message == "":
             break

test/test_urlutils.cpp

@@ -20,17 +20,35 @@
 
 #include <QDebug>
 #include <QFileInfo>
+#include <QTemporaryFile>
+#include <QUrl>
 
 
 TEST_CASE( "UrlUtils" )
 {
-  // should be considered relative
-  REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "path/to/file" ) ) );
-  REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "/path/to/file" ) ) );
-  REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "file:///path/to/file" ) ) );
-
-  // should NOT be considered relative
-  REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "http://osm.org" ) ) );
-  REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "http://osm.org/test?query=1" ) ) );
-  REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "https://osm.org/test?query=1" ) ) );
+  SECTION( "isRelativeOrFileUrl" )
+  {
+    // should be considered relative
+    REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "path/to/file" ) ) );
+    REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "/path/to/file" ) ) );
+    REQUIRE( UrlUtils::isRelativeOrFileUrl( QStringLiteral( "file:///path/to/file" ) ) );
+
+    // should NOT be considered relative
+    REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "http://osm.org" ) ) );
+    REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "http://osm.org/test?query=1" ) ) );
+    REQUIRE( !UrlUtils::isRelativeOrFileUrl( QStringLiteral( "https://osm.org/test?query=1" ) ) );
+  }
+
+  SECTION( "fromString" )
+  {
+    // a file that exists will be transformed into a file:// URL
+    QTemporaryFile tmpFile( QStringLiteral( "test.jpg" ) );
+    REQUIRE( UrlUtils::fromString( tmpFile.fileName() ).toString() == QUrl::fromLocalFile( tmpFile.fileName() ).toString() );
+
+    // a string that doesn't link to an existing file will not transform into a file:// URL
+    REQUIRE( UrlUtils::fromString( QStringLiteral( "/my/missing/file.txt" ) ).toString() == QStringLiteral( "/my/missing/file.txt" ) );
+
+    // a URL string (e.g. http(s)) will be handled as such
+    REQUIRE( UrlUtils::fromString( QStringLiteral( "https://www.opengis.ch/" ) ).toString() == QStringLiteral( "https://www.opengis.ch/" ) );
+  }
 }