(In reply to :Gijs (he/him) from comment #4) > This seems to be a result of the code at https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/toolkit/components/extensions/parent/ext-downloads.js#581-584 rejecting any sanitized paths. I'm not sure why it does that. Presumably the webextension folks woul dknow. The idea is that if extension used a filename with invalid characters, we would throw. Since this is an actual api with explicitly passed filename, we prefer throwing an error instead of silently failing or normalizing it. We detect errors by checking if sanitized filename differs from the passed argument. This has mostly worked as described until bug 1598216. > The double space sanitization got added in bug 1598216 so that if we get sent a path with `foo :bar` in it, and we sub a space for `:`, we normalize the two consecutive spaces back to 1 space... That sounds like good reasoning for the case when the original filename is invalid, but it looks like consecutive spaces normalization was intentional for already valid filenames. This is definitly not what we want for an explicit api lie extensions, but I would also argue Firefox shouldn't be dropping consecutive spaces from otherwise valid filenames, they might be meaningful in some cases (just an anecdote, but my partner used to regularly download reports from a local government website which used significant leading whitespace for dates and case numbers, for proper sorting purposes). Mak, what do you think?
Bug 1637973 Comment 5 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to :Gijs (he/him) from comment #4) > This seems to be a result of the code at https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/toolkit/components/extensions/parent/ext-downloads.js#581-584 rejecting any sanitized paths. I'm not sure why it does that. Presumably the webextension folks woul dknow. The idea is that if extension used a filename with invalid characters, we would throw. Since this is an actual api with explicitly passed filename, we prefer throwing an error instead of silently failing or normalizing it. We detect errors by checking if sanitized filename differs from the passed argument. This has mostly worked as described until bug 1598216. > The double space sanitization got added in bug 1598216 so that if we get sent a path with `foo :bar` in it, and we sub a space for `:`, we normalize the two consecutive spaces back to 1 space... That sounds like good reasoning for the case when the original filename is invalid, but it doesn't looks like consecutive spaces normalization was intentional for already valid filenames. This is definitely not what we want for an explicit api like extensions, but I would also argue Firefox shouldn't be dropping consecutive spaces from otherwise valid filenames, they might be meaningful in some cases (just an anecdote, but my partner used to regularly download reports from a local government website which used leading whitespace for dates and case numbers, for proper sorting purposes). Mak, what do you think?
(In reply to :Gijs (he/him) from comment #4) > This seems to be a result of the code at https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/toolkit/components/extensions/parent/ext-downloads.js#581-584 rejecting any sanitized paths. I'm not sure why it does that. Presumably the webextension folks woul dknow. The idea is that if extension used a filename with invalid characters, we would throw. Since this is an actual api with explicitly passed filename, we prefer throwing an error instead of silently failing or normalizing it. We detect errors by checking if sanitized filename differs from the passed argument. This has mostly worked as described until bug 1598216. > The double space sanitization got added in bug 1598216 so that if we get sent a path with `foo :bar` in it, and we sub a space for `:`, we normalize the two consecutive spaces back to 1 space... That sounds like good reasoning for the case when the original filename is invalid, but it doesn't looks like consecutive spaces normalization was intentional for already valid filenames. This is definitely not what we want for an explicit api like extensions, but I would also argue Firefox shouldn't be dropping consecutive spaces from otherwise valid filenames, they might be meaningful in some cases (just an anecdote, but my partner used to regularly download reports from a local government website which used leading whitespace for dates and case numbers, for proper sorting purposes). Marco, what do you think?