1562176 - "Save Page As" should do something about colons in the page title on non-mac/windows/android

Status: VERIFIED FIXED

(Firefox :: File Handling, defect, P5)

Description

[Mike Hommey [:glandium]]

"Save Page As" takes a page's title as file name. The file name is also used to store extra resources.

So when you save a page with the title "Foo bar", you end up with a "Foo bar.html" file, and a "Foo bar files" directory containing the resources of the html page. Links in the html are written as relative paths starting with the directory name.

Now, here's how things can go badly:

• Open about:memory. Its page title is "about:memory"
• Save the page as, and keep the default file name, which is "about:memory.xhtml"
• If you check the directory where the file was saved, you'll also notice a "about:memory_files" directory.
• If you open the "about:memory.xhtml" file, it will be unstyled. Why? Because the link to the stylesheet is literally "about:memory_files/aboutMemory.css", so the browser is actually trying to open the corresponding about: url that doesn't exist, rather than the relatively situated file.

I don't think there are ways to exploit this, because of the addition of the _files suffix, but this can be an inconvenience for users saving pages, where the associated resources are not accessed as expected.

Colons could be transformed into underscores for the file name, or the links could be rewritten to start with "./". The latter would seem cleaner to me.

Comment 1

[ovidiu boca[:Ovidiu]]

I think this should go under File Handling, please feel free to change it if you consider that a different component is more suitable for this issue.
Also reproducible on Nightly 69.0a1(2019-07-01) build.

Comment 2

[:Gijs (he/him)]

Sigh. This isn't an issue on mac/windows/android.

Comment 3

[:Gijs (he/him)]

(In reply to Mike Hommey [:glandium] from comment #0)

The links could be rewritten to start with "./". [that] would seem cleaner to me.

I dunno, it's not necessary for 99.99% of relative links so doing it everywhere seems overkill. Doing it only where necessary in the webbrowserpersist code (which is generating the broken links here when we're using platform-valid filenames that include colons on Linux/BSD/...) is tricky. If you feel strongly, please file another bug - in Core :: DOM (which is where that code lives, for better or worse).

I'm just going to fix the frontend suggested filename sanitization to exclude colons everywhere (replacing with a space because that's what we do on all the platforms that already exclude it) to avoid this issue.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1562176 - fix colons in 'save file as' filenames, r?mak

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9206e361572
fix colons in 'save file as' filenames, r=mak

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on test_DownloadPaths.js

backout: https://hg.mozilla.org/integration/autoland/rev/c58773dc939aab5b059fd3335b1d2513edc8224a

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=255505753&revision=c9206e361572d5b404fc34590793f87ee4592703

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255505753&repo=autoland&lineNumber=2772

[task 2019-07-09T16:22:30.034Z] 16:22:30 INFO - Retrying tests that failed when run in parallel.
[task 2019-07-09T16:22:30.042Z] 16:22:30 INFO - TEST-START | toolkit/components/downloads/test/unit/test_DownloadPaths.js
[task 2019-07-09T16:22:30.402Z] 16:22:30 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/downloads/test/unit/test_DownloadPaths.js | xpcshell return code: 0
[task 2019-07-09T16:22:30.404Z] 16:22:30 INFO - TEST-INFO took 359ms
[task 2019-07-09T16:22:30.405Z] 16:22:30 INFO - >>>>>>>
[task 2019-07-09T16:22:30.406Z] 16:22:30 INFO - PID 30152 | WARN 2019-07-09T16:22:30Z: audio_thread_priority::rt_linux: Could not make thread real-time.
[task 2019-07-09T16:22:30.407Z] 16:22:30 INFO - PID 30152 | [30152, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2555
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - PID 30152 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - PID 30152 | [30152, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/extensions/permissions/nsPermissionManager.cpp, line 2903
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2019-07-09T16:22:30.408Z] 16:22:30 INFO - running event loop
[task 2019-07-09T16:22:30.410Z] 16:22:30 INFO - PID 30152 | [30152, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/workspace/build/src/dom/media/CubebUtils.cpp, line 391
[task 2019-07-09T16:22:30.411Z] 16:22:30 INFO - "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
[task 2019-07-09T16:22:30.412Z] 16:22:30 INFO - toolkit/components/downloads/test/unit/test_DownloadPaths.js | Starting test_common_initialize
[task 2019-07-09T16:22:30.413Z] 16:22:30 INFO - (xpcshell/head.js) | test test_common_initialize pending (2)
[task 2019-07-09T16:22:30.414Z] 16:22:30 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2019-07-09T16:22:30.415Z] 16:22:30 INFO - (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2019-07-09T16:22:30.416Z] 16:22:30 INFO - (xpcshell/head.js) | test test_common_initialize finished (2)
[task 2019-07-09T16:22:30.418Z] 16:22:30 INFO - toolkit/components/downloads/test/unit/test_DownloadPaths.js | Starting test_sanitize
[task 2019-07-09T16:22:30.420Z] 16:22:30 INFO - (xpcshell/head.js) | test test_sanitize pending (2)
[task 2019-07-09T16:22:30.421Z] 16:22:30 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/downloads/test/unit/test_DownloadPaths.js | test_sanitize - [test_sanitize : 9] "A ?|\"\"<<>>;,+=[]B][=+,;>><<\"\"|? C" == "A:?|\"\"<<>>;,+=[]B][=+,;>><<\"\"|?:C"
[task 2019-07-09T16:22:30.423Z] 16:22:30 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/toolkit/components/downloads/test/unit/test_DownloadPaths.js:testSanitize:9

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4f105259931
fix colons in 'save file as' filenames, r=mak

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/b4f105259931

Comment 9

[Negritas Sergiu, Desktop QA]

I have managed to reproduce the bug following the STR from the first comment on Fx 69.0a1 (2019-06-18). I can confirm that the issue is fixed on Fx 70.0b8 using both versions of Fx on Ubuntu 18.04 LTS.