Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Disallow file and folder names that end with dots #7772

Merged
merged 3 commits into from
May 20, 2014
Merged

Disallow file and folder names that end with dots #7772

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68bc4f9
update regex
JeffryBooher May 5, 2014
baa4ab0
disallow trailing dots in file/directory names, rework error messages…
JeffryBooher May 7, 2014
631c7ca
fix lint error, rearrange error message and emphasize invalid characters
JeffryBooher May 15, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ define({
"FILE_EXISTS_ERR" : "The file or directory already exists.",
"FILE" : "file",
"DIRECTORY" : "directory",
"DIRECTORY_NAMES_LEDE" : "Directory names",
"FILENAMES_LEDE" : "Filenames",
"FILENAME" : "filename",
"DIRECTORY_NAME" : "directory name",


// Project error strings
"ERROR_LOADING_PROJECT" : "Error loading project",
Expand All @@ -59,9 +64,9 @@ define({
"ERROR_RENAMING_FILE" : "An error occurred when trying to rename the file <span class='dialog-filename'>{0}</span>. {1}",
"ERROR_DELETING_FILE_TITLE" : "Error deleting file",
"ERROR_DELETING_FILE" : "An error occurred when trying to delete the file <span class='dialog-filename'>{0}</span>. {1}",
"INVALID_FILENAME_TITLE" : "Invalid {0} name",
"INVALID_FILENAME_MESSAGE" : "Filenames cannot contain the following characters: {0} or use any system reserved words.",
"FILE_ALREADY_EXISTS" : "The {0} <span class='dialog-filename'>{1}</span> already exists.",
"INVALID_FILENAME_TITLE" : "Invalid {0}",
"INVALID_FILENAME_MESSAGE" : "{0} cannot contain the following characters: {1}, use any system reserved words or end with dots (.).",
"ENTRY_WITH_SAME_NAME_EXISTS" : "A file or directory with the name <span class='dialog-filename'>{0}</span> already exists.",
"ERROR_CREATING_FILE_TITLE" : "Error creating {0}",
"ERROR_CREATING_FILE" : "An error occurred when trying to create the {0} <span class='dialog-filename'>{1}</span>. {2}",

Expand Down
17 changes: 8 additions & 9 deletions src/project/ProjectManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ define(function (require, exports, module) {
* RegEx to validate if a filename is not allowed even if the system allows it.
* This is done to prevent cross-platform issues.
*/
var _illegalFilenamesRegEx = /^(\.+|com[1-9]|lpt[1-9]|nul|con|prn|aux)$/i;

var _illegalFilenamesRegEx = /^(\.+|com[1-9]|lpt[1-9]|nul|con|prn|aux|)$|\.+$/i;

var suppressToggleOpen = false;

Expand Down Expand Up @@ -1529,8 +1530,8 @@ define(function (require, exports, module) {
filename.match(_illegalFilenamesRegEx)) {
Dialogs.showModalDialog(
DefaultDialogs.DIALOG_ID_ERROR,
StringUtils.format(Strings.INVALID_FILENAME_TITLE, isFolder ? Strings.DIRECTORY : Strings.FILE),
StringUtils.format(Strings.INVALID_FILENAME_MESSAGE, _invalidChars)
StringUtils.format(Strings.INVALID_FILENAME_TITLE, isFolder ? Strings.DIRECTORY_NAME : Strings.FILENAME),
StringUtils.format(Strings.INVALID_FILENAME_MESSAGE, isFolder ? Strings.DIRECTORY_NAMES_LEDE : Strings.FILENAMES_LEDE, _invalidChars)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like a bunch of these string diffs are just here to change the dialog message from "file name" to "filename." I think "file name" is actually better -- and it seems like it would simplify the diff too.

CC @njx for the string question

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very used to "filename" (and that's how we tend to treat it in code - we don't inner-cap the N). For user-facing strings "file name" is probably more standard English, though for technical users it might be different - I have a suspicion that coders are probably more used to "filename", though I don't have any data.

(FWIW, I did a search in Adobe's help docs, and it seems like the documentation itself usually uses "filename", but when it refers to label strings in the UI they tend to be "File Name", probably because they need to be capitalized and "Filename" looks weird. That doesn't apply to this case anyway since it's uncapitalized in a message string, not being used as a label.)

Anyway, I guess after all that I don't have a strong opinion either way :), as long as we're consistent. I didn't see any other user-visible uses of "filename" in the current strings file, so I think this one could go either way, and we can follow it elsewhere. Personally, I think it's fine to keep it as "filename".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@njx Actually it looks like all of these should be uppercase, since they're being used in a dialog title. I think it's a bug in the PR (and on master!) that they're lowercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterflynn Well, I changed this because you're right we should be Title Case on the titles but having trouble testing it. I tried removing all permissions on a folder except for "Read" and renaming a file I ran into "Error renaming file" then I noticed we also have "Error deleting file", "Error loading project". I'm not sure how deep this issue goes and it could go kind of deep.

We could try to fix all of them, just this occurrence or raise an issue to fix them all at once. We could attempt to fix them using a css text-transform (text-transform:capitalize) but I have no idea how that works with localization. It may not work or not work it incorrectly in some scenarios.

);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of hard to read the list of invalid chars on my Windows HiDPI laptop monitor because they're kind of small and jammed together. What do you think about having a separate _invalidCharsDisplayString that has a space between each char? Or use boldface? Or both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redmunds putting a space in means that we maintain 2 strings whenever we update it. Bold works b/c we can just write the format around it. Let's do that and see how it looks on your machine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use _invalidChars.split("").join(" ") to add spaces, or use replace with a regular expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... or we could use a fixed width font and bold it. try it now @redmunds

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me.

return false;
}
Expand Down Expand Up @@ -1657,15 +1658,13 @@ define(function (require, exports, module) {
};

var errorCallback = function (error, entry) {
var entryType = isFolder ? Strings.DIRECTORY : Strings.FILE,
oppositeEntryType = isFolder ? Strings.FILE : Strings.DIRECTORY;
var titleType = isFolder ? Strings.DIRECTORY_NAME : Strings.FILENAME,
entryType = isFolder ? Strings.DIRECTORY : Strings.FILE;
if (error === FileSystemError.ALREADY_EXISTS) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice simplification! (Using a more generic string and eliminating the "opposite type" stuff here).

var useOppositeType = (isFolder === entry.isFile);
Dialogs.showModalDialog(
DefaultDialogs.DIALOG_ID_ERROR,
StringUtils.format(Strings.INVALID_FILENAME_TITLE, entryType),
StringUtils.format(Strings.FILE_ALREADY_EXISTS,
useOppositeType ? oppositeEntryType : entryType,
StringUtils.format(Strings.INVALID_FILENAME_TITLE, titleType),
StringUtils.format(Strings.ENTRY_WITH_SAME_NAME_EXISTS,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSLint error:

1667    Unexpected '(space)'.    StringUtils.format(Strings.ENTRY_WITH_SAME_NAME_EXISTS,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using The Interactive Linter and it didn't flag this as an error. I opened it again and it still says that it's good. I think that it's using JSHint instead of JSLint which is more lenient. I need to figure out where there interactive-linter options are and change this. Sorry about that.

StringUtils.breakableUrl(data.rslt.name))
);
} else {
Expand Down