helpers: Include filename when triggering download

By combining the two old ideas: Instead of deciding between clicking
on a link (needs CSP), or loading a "data:" URL into a nested
iframe (omits filename), we click on a link in a nested iframe.

This should make the "Launch viewer" button work with Chrome and in
other contexts where the extension of the filename is needed to find
the right external application.

Fixes #1952

src/helpers.js

@@ -296,37 +296,48 @@ export function rephraseUI(key, original) {
  * @param mimeType
  * @returns {*}
  */
-export function fileDownload({ data, _fileName = 'myFile.dat', mimeType = 'application/octet-stream' }) {
+export function fileDownload({ data, fileName = 'myFile.dat', mimeType = 'application/octet-stream' }) {
     if (!data) {
         console.error('fileDownload(): no data to download');
         return false;
     }
 
-    const a = document.createElement('a');
-    a.id = 'dynamically-generated-file';
-    a.href = `data:${mimeType},${encodeURIComponent(data)}`;
-    document.body.appendChild(a); // if not used further then at least within integration tests
-
-    // Workaround since I can't get CSP working for this
-    /*
-    if ('download' in a) { // html5 A[download]
-        logDebug('fileDownload() is using A.HREF');
+    // It is important to tell the filename to the browser, especially
+    // the extension. That makes mime-type detection work, which seems
+    // to be necessary for at least Chrome. It also produces files in
+    // the Download folder of the user that make sense later.
+    //
+    // So we want to trigger the download by clicking on a link like
+    // this:
+    //
+    //   <a href="data:<mime>,<data>" download="<filename.ext>" />
+    //
+    // This, however, needs a CSP of "frame-src data:" in our parent
+    // frame (the Shell), so that the browser allows 'navigating' to
+    // the "data:" URL in our iframe.
+    //
+    // But we can instead create our own custom iframe inside
+    // ourselves and put the link inside that. This nested iframe also
+    // needs a CSP of "frame-src data:", but now it is us that defines
+    // the policy, via our manifest. No changes to the Shell are
+    // needed.
+
+    const f = document.createElement('iframe');
+    f.setAttribute("hidden", "hidden");
+    f.addEventListener("load", () => {
+        // Once we get the "load" event, we can start modifying the
+        // document inside the iframe.
+        const doc = f.contentDocument;
+        const a = doc.createElement('a');
+        a.href = `data:${mimeType},${encodeURIComponent(data)}`;
         a.setAttribute('download', fileName);
+        doc.body.appendChild(a);
         a.click();
-    } else */ { // do iframe dataURL download
-        logDebug('fileDownload() is using IFRAME');
-        const f = document.createElement('iframe');
-        document.body.appendChild(f);
-        f.setAttribute("hidden", "hidden");
-        const nicerText = '\n[...............................GraphicsConsole]\n';
-        f.src = `data:${mimeType},${encodeURIComponent(data + nicerText)}`;
         window.setTimeout(() => document.body.removeChild(f), 333);
-    }
+    });
 
-    window.setTimeout(() => { // give test browser some time ...
-        logDebug('removing temporary A.HREF for filedownload');
-        document.body.removeChild(a);
-    }, 5000);
+    // Start the show
+    document.body.appendChild(f);
     return true;
 }
 

test/check-machines-consoles

@@ -29,9 +29,8 @@ class TestMachinesConsoles(machineslib.VirtualMachinesCase):
 
     def waitDownloadFile(self, filename: str, expected_size: int | None = None, content: str | None = None) -> None:
         filepath = self.browser.driver.download_dir / filename
+        testlib.wait(filepath.exists)
 
-        # Big downloads can take a while
-        testlib.wait(filepath.exists, tries=120)
         if expected_size is not None:
             testlib.wait(lambda: filepath.stat().st_size == expected_size)
 
@@ -49,24 +48,8 @@ host={host}
 port={port}
 delete-this-file=1
 fullscreen=0
-
-[...............................GraphicsConsole]
 """
-        # HACK: Due to a bug in cockpit-machines, Firefox downloads the desktop
-        # viewer file as a randomly generated file while chromium as
-        # "download". As we want to download this as "$vmName.vv" in the end
-        # work around the issue for now.
-
-        def ready_downloads():
-            return list(filter(lambda p: not p.endswith(".part"), os.listdir(self.browser.driver.download_dir)))
-
-        if self.browser.browser == "chromium":
-            fname = "download"
-        else:
-            testlib.wait(lambda: len(ready_downloads()) == 1)
-            fname = ready_downloads()[0]
-
-        self.waitDownloadFile(fname, content=content, expected_size=len(content))
+        self.waitDownloadFile("console.vv", content=content, expected_size=len(content))
 
     @testlib.skipImage('SPICE not supported on RHEL', "rhel-*", "centos-*")
     def testExternalConsole(self):