Agreed; that seems like the kind of thing that should really be delegated.

Created attachment 93012 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems

Comment on attachment 93012 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems lgtm, thanks.

Comment on attachment 93012 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems View in context: https://bugs.webkit.org/attachment.cgi?id=93012&action=review > Source/WebCore/ChangeLog:10 > + No new tests. (OOPS!) This will make the commit-queue fail. You should explain why there are no new tests (is testing impossible here?) or you should add tests. Also, the FileSystem feature should be announced on webkit-dev per the new feature guidelines: http://www.webkit.org/coding/adding-features.html

Comment on attachment 93012 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems View in context: https://bugs.webkit.org/attachment.cgi?id=93012&action=review > Source/WebCore/fileapi/DOMFileSystemBase.cpp:148 > + if (type != AsyncFileSystem::External && !DOMFilePath::isValidPath(absolutePath)) if ((type == Temporary || type == Persistent) && !DOMFilePath::isValidPath()) might be better? This is for basic checks that are explicitly noted in the spec (draft), therefore performing the checks for FS types that are spec'ed out would make more sense.

(In reply to comment #4) > (From update of attachment 93012 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93012&action=review > > > Source/WebCore/ChangeLog:10 > > + No new tests. (OOPS!) > > This will make the commit-queue fail. You should explain why there are no new tests (is testing impossible here?) or you should add tests. > > Also, the FileSystem feature should be announced on webkit-dev per the new feature guidelines: > http://www.webkit.org/coding/adding-features.html Oh, is the guideline applied to the features that are already being experimented/tested? Well it'd be better to make this experimental feature more visible and to request more comments... Ok I'll be sending out an announce to webkit-dev.

Created attachment 93156 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems addressed comments from the previous review cycle

Comment on attachment 93012 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems View in context: https://bugs.webkit.org/attachment.cgi?id=93012&action=review >>> Source/WebCore/ChangeLog:10 >>> + No new tests. (OOPS!) >> >> This will make the commit-queue fail. You should explain why there are no new tests (is testing impossible here?) or you should add tests. >> >> Also, the FileSystem feature should be announced on webkit-dev per the new feature guidelines: >> http://www.webkit.org/coding/adding-features.html > > Oh, is the guideline applied to the features that are already being experimented/tested? Well it'd be better to make this experimental feature more visible and to request more comments... > Ok I'll be sending out an announce to webkit-dev. done >> Source/WebCore/fileapi/DOMFileSystemBase.cpp:148 >> + if (type != AsyncFileSystem::External && !DOMFilePath::isValidPath(absolutePath)) > > if ((type == Temporary || type == Persistent) && !DOMFilePath::isValidPath()) > > might be better? This is for basic checks that are explicitly noted in the spec (draft), therefore performing the checks for FS types that are spec'ed out would make more sense. done

The commit-queue encountered the following flaky tests while processing attachment 93156 [details]: http/tests/xmlhttprequest/remember-bad-password.html bug 51733 (author: ap@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 93156 [details] Prevented DOMFileSystemBase from imposing file naming restrictions on external file systems Clearing flags on attachment: 93156 Committed r86291: <http://trac.webkit.org/changeset/86291>

All reviewed patches have been landed. Closing bug.

http://trac.webkit.org/changeset/86291 might have broken GTK Linux 32-bit Release