Bug 58517 – Vendor-prefix requestFileSystem in FileSystem API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kinuko Yasuda]

Since FileSystem API is not standard yet and will likely change, we should vendor-prefix the exposed API.

Specifically:
window.requestFileSystem should be renamed to window.webkitRequestFileSystem.

[Kinuko Yasuda]

I guess following APIs should also be prefixed?

window.resolveLocalFileSystemURL

WorkerContext.requestFileSystem
WorkerContext.requestFileSystemSync
WorkerContext.resolveLocalFileSystemURL
WorkerContext.resolveLocalFileSystemSyncURL

[Taiju Tsuiki]

Created attachment 89557 [details]
Patch

[Kinuko Yasuda]

Comment on attachment 89557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89557&action=review

> Source/WebCore/ChangeLog:6
> +        https://bugs.webkit.org/show_bug.cgi?id=58517

Looks good, thanks for working on this.
I think it'd be better to add some explanation why and how (which method) we're prefixing in the ChangeLog.
(Probably you can just copy and paste the explanation from the bug.)

[Darin Fisher (:fishd, Google)]

Comment on attachment 89557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89557&action=review

> Source/WebCore/page/DOMWindow.idl:200
>          attribute [EnabledAtRuntime=FileSystem] FlagsConstructor Flags;

What about Flags?  Should this be WebKitFlags for completeness?

(I think it is OK to leave TEMPORARY and PERSISTENT as is... just not sure about Flags.)

[Eric U.]

Comment on attachment 89557 [details]
Patch

What about the flags TEMPORARY and PERSISTENT?  Should we prefix them too?

[Eric U.]

(In reply to comment #5)
> (From update of attachment 89557 [details])
> What about the flags TEMPORARY and PERSISTENT?  Should we prefix them too?

Sorry--missed your comment.  Nevermind.

[Taiju Tsuiki]

Created attachment 89982 [details]
Patch

[WebKit Review Bot]

Attachment 89982 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8459821

[Build Bot]

Attachment 89982 [details] did not build on win:
Build output: http://queues.webkit.org/results/8459825

[Early Warning System Bot]

Attachment 89982 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8435870

[Taiju Tsuiki]

Created attachment 89992 [details]
Patch

[WebKit Review Bot]

Attachment 89992 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8442874

[Build Bot]

Attachment 89992 [details] did not build on win:
Build output: http://queues.webkit.org/results/8459854

[Taiju Tsuiki]

Created attachment 89995 [details]
Patch

[Taiju Tsuiki]

Created attachment 89996 [details]
Patch

[Taiju Tsuiki]

Created attachment 89999 [details]
Patch

[Kinuko Yasuda]

(In reply to comment #16)
> Created an attachment (id=89999) [details]
> Patch

Thanks for working on this.  The patch looks good to me.
(Could any 'real' reviewer take a look at the patch?)

[WebKit Commit Bot]

Comment on attachment 89999 [details]
Patch

Clearing flags on attachment: 89999

Committed r84224: <http://trac.webkit.org/changeset/84224>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.