Bugzilla – Bug 119
Linux: Export Multiple: "*" and "?" in label or track name wrongly rejected
Last modified: 2021-02-06 11:38:34 UTC
On Linux, "/" is the file separator and illegal. It should be trapped and display our "not a legal filename" dialogue. Instead, it gives a confusing "cannot export audio to" message. On Mac, "/" is also the file separator but is translated internally by the OS so should be accepted. It succeeds when used in Audacity's Save/Export dialogues, but gives the same "cannot export audio to" error in Export Multiple. On both OS'es, "*" and "?" are trapped by the "not a legal filename" dialogue when they are perfectly legal.
Promoting to P3 so that it can have a release note. It's quite often complained about.
Created attachment 182 [details] patch from Seshadri Mahalingam to handle export multiple chars correctly
(In reply to comment #2) Seshadri, I did look at/try the patch on Ubuntu - though I changed #if defined (__LINUX__) to #if defined (__WXGTK__). I am not on Mac. The only Linux change is to trap "/" as illegal so display the illegal filenames dialogue instead of the spurious error about "cannot write". That works in my test but I don't think it's a perfect fix because the dialogue checks wxFileName::GetForbiddenChars for its list of forbidden characters. So the dialogue still states that you cannot use any of *? - it should say you cannot use any of /*?. I don't know though exactly where the list is or understand why "/" is not on the list in the first place. Similarly the patch does not address the part of the bug that "*" and "?" are trapped on Mac and Linux by the dialogue. They should not be on the forbidden characters list for Mac or Linux. What happens on Mac after your patch if you type "/" in a label? It looks as if that would trigger the error dialogue, but if I understand it, "/" is permitted when typing the file name in normal export because of the translation to ":".
Created attachment 184 [details] Patch for bug 119 Thanks for trying this out, Gale! This patch and the previous one bypass the "*" and "?" restriction for both OS X and Linux. The dialog after applying the previous patch incorrectly stated that * and ? were not allowed on either operating system. The 'Export Multiple' sequence used wxFileName::GetForbiddenChars (http://docs.wxwidgets.org/stable/wx_wxfilename.html#wxfilenamegetforbiddenchars) which excludes "*" and "?" on all UNIXy operating systems. The non-trivial part of the previous patch that I ought to have documented better handles "/" on Macs by replacing every occurrence of "/" with ":". The colon is the Classic path separating character and `touch f:b` for example will create a file denoted by Finder (the file explorer) as "f/b". To the UNIX layer however, the "/" character is seen as a ":". OS X does not permit ":" in a file name (probably due to historic reasons). The new patch attains the desired functionality and makes the dialog more accurate by abstracting the forbidden character detection into an Audacity function. On a Mac, any label that doesn't contain a ":" character is permitted. On Ubuntu/Linux (untested!) only labels with a "/" should throw the dialog. "*" and "?" should now be permitted characters on both operating systems.
(In reply to comment #4) Thanks, Seshadri for the comments. I'll be glad to try your new patch on Ubuntu, but the patch you posted today is actually the same as the one I originally posted for you. With your patch (if I now understand it) if user on Mac enters "/", Audacity will accept it - the translation to ":" and back to "/" (as it appears in Finder) will all be done "under the hood"? I still don't quite understand why the Widgets list forbids "?" and "*" on Mac and Linux and forbids "/" on Mac. Is that a bug that should be corrected upstream or is there a reason it's the way it is? Meantime I changed bug status back to "REOPENED" - our current workflow is only to move to DEVEL - FIX MADE when a patch has been committed. I made this clearer now on http://wiki.audacityteam.org/wiki/Bug_Lists#Workflow_when_resolving_bugs . Since I'm not actually a programmer, a developer will still need to review the patch for being "code appropriate" and take the decision to commit it.
Created attachment 185 [details] Updated patch Whoops, sorry about the double-patch :S The attached patch is the amended one that I referred to in my previous comment. > the translation to ":" and back to "/" (as it appears in > Finder) will all be done "under the hood" Yes, that's right. However, with this patch, when the user sees the success dialog that lists file paths, they may see the The wx source for GetForbiddenChars is available at: http://trac.wxwidgets.org/browser/wxWidgets/trunk/src/common/filename.cpp#L1760 The wxPATH_MAC case is misleading and applies only to Classic, pre-OS X Macs. On OS X the wxPATH_UNIX flag is on, so wx defaults to claiming * and ? are invalid. This is incorrect under the UNIX, Linux and OS X platforms, and I think should probably be corrected upstream. In the meantime, this patch should correct Audacity's behaviour.
(In reply to comment #6) Thanks, Seshadri. It seems to work fine on Ubuntu 10.10 now. One small point is that I believe the illegal filenames dialogue on Mac now says "You cannot use any of: :". On Linux it says "You cannot use any of: /". However on Ubuntu the bottom of that "/" character makes it seem there is no space between ":" and "/" so it looks like "cannot use any of:/". I think it could still confuse on Mac even if the space between the two colons is evident. I'd propose changing this to "You cannot use %s characters.\nUse...". If we apply the patch and don't want to break translations for 2.0 I suggest this could be marked as a TODO. >> the translation to ":" and back to "/" (as it appears in Finder) will all >> be done "under the hood" > Yes, that's right. However, with this patch, when the user sees the success > dialog that lists file paths, they may see the Can you complete that sentence? > The wxPATH_MAC case is misleading and applies only to Classic, pre-OS X Macs. > On OS X the wxPATH_UNIX flag is on, so wx defaults to claiming * and ? are > invalid. This is incorrect under the UNIX, Linux and OS X platforms, and I > think should probably be corrected upstream. +1.
(In reply to comment #3) > The only Linux change is to trap "/" as illegal so display the illegal > filenames dialogue instead of the spurious error about "cannot write". That > works in my test but I don't think it's a perfect fix because the dialogue > checks wxFileName::GetForbiddenChars for its list of forbidden characters. Right. I've reviewed the patch, and I think it's better to get the result from wxFileName::GetForbiddenChars() (which catches several the patch does not). The right approach is to let wxWidgets changes help us, and modify the string if necessary. In fact, I see this was reported 2010-02-21, and the patch in July 2011. I believe our upgrade to wxWidgets 2.8.12 fixed at least part of this. wxFileName::GetForbiddenChars() in 2.8.12: * Returns wxT("*?") for all platforms except wxPATH_MAC and wxPATH_DOS. * Returns wxT("") for Mac, and has an explicit comment that * and ? are allowed. * Returns wxT("*?\\/:\"<>|" for wxPATH_DOS. So is the only change we currently need, to add to the result for wxPATH_UNIX, so it's wxT("*?/")? Or that plus make the Mac forbiddens be wxT(":")?
Please evaluate this fix: 02ce3c312bb63874565dcfc311f842341a8f539a
(In reply to Paul L from comment #9) Hi Paul Thanks for all the fixes. Pasting links to fixes - not just a revision # - would be greatly appreciated.
I thought my old habit of giving links to GitHub was more care than was expected, that a commit id was enough. But if you think otherwise, very well then: https://github.com/audacity/audacity/commit/02ce3c312bb63874565dcfc311f842341a8f539a
(In reply to Paul L from comment #11) Tests ok on Mac El Capitan d6187f6 21Jun16
Behavior is different on Windows - here the "/", "*" or "?" gets replaced with an underscore "_" in the filename. Or the user is offered the chance to alter it to a legally valid filename Manually . Note that the "/", "*" or "?" are retained for the metadata "Title" - this is correct behavior I believe.
Are you saying any part of the Windows behavior is incorrect? I believe none of it is wrong.
(In reply to Paul L from comment #14) >Are you saying any part of the Windows behavior is incorrect? No, just clumsily worded - I too think the Windows behavior is correct
Thanks Paul. Yes I think Windows behavior in choosing what characters are valid or not remains correct. Seems OK on Mac but the problem is not corrected on Linux. We still reject * and ? in Export Multiple as at "c9422aa" 08Jul16, so REOPENED. The only character we want to reject on Linux is the forward slash. I want to request a tweak to the Save As... error dialogue when an illegal character is encountered. At present we have the string "You cannot use any of:" but the colon in that string is confusing when colon is valid, as it is on Linux, and on Mac, where colon is the only rejection. I suggest "You cannot use any of these".
Also on Linux the Save As... dialogue sometimes shows repeats of the invalid characters e.g. cannot use any of:*?/*?/*?/*?/ I got it frequently after experimenting with cases where tracks and labels contain characters that Audacity rejects. I saved a project for such a case, which save of course crashed due to bug 1397. When I recovered the project, and tried Export Multiple, the Save As... dialogue only showed one set of invalid characters. Something to bear in mind when this is fixed for Linux.
(In reply to Gale Andrews from comment #16) > I want to request a tweak to the Save As... error dialogue when an illegal > character is encountered. At present we have the string "You cannot use > any of:" but the colon in that string is confusing when colon is valid, as > it is on Linux, and on Mac, where colon is the only rejection. James has fixed this at https://github.com/audacity/audacity/commit/b56ea05 . That fix should work on Linux when this bug is fixed on Linux.
(In reply to Gale Andrews from comment #17) > Also on Linux the Save As... dialogue sometimes shows repeats of the invalid > characters e.g. > > cannot use any of:*?/*?/*?/*?/ > > I got it frequently after experimenting with cases where tracks and labels > contain characters that Audacity rejects I have now seen it twice on Windows too.
Updated "Hardware", bug title and Release Note to be Linux only.
Fix to bug as originally stated: https://github.com/audacity/audacity/commit/7548388 And I reworded the message dialog a bit to make the invalid character to stand out better. However, I was unable to reproduce Gale's mentioned in comments 17 and 19.
Tested on linux too. Works fine. Suggests changing "/" to "_" and does so. RESOLVED FIXED
