Bug 58517 - Vendor-prefix requestFileSystem in FileSystem API
: Vendor-prefix requestFileSystem in FileSystem API
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-13 23:25 PDT by Kinuko Yasuda
Modified: 2011-04-18 22:37 PDT (History)
8 users (show)

See Also:


Attachments
Patch (40.37 KB, patch)
2011-04-14 05:10 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (83.72 KB, patch)
2011-04-17 19:02 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (83.78 KB, patch)
2011-04-17 22:01 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (250.87 KB, patch)
2011-04-17 22:58 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (89.49 KB, patch)
2011-04-17 23:06 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (90.41 KB, patch)
2011-04-18 00:20 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2011-04-13 23:25:05 PDT
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.
Comment 1 Kinuko Yasuda 2011-04-13 23:36:23 PDT
I guess following APIs should also be prefixed?

window.resolveLocalFileSystemURL

WorkerContext.requestFileSystem
WorkerContext.requestFileSystemSync
WorkerContext.resolveLocalFileSystemURL
WorkerContext.resolveLocalFileSystemSyncURL
Comment 2 Taiju Tsuiki 2011-04-14 05:10:23 PDT
Created attachment 89557 [details]
Patch
Comment 3 Kinuko Yasuda 2011-04-14 06:14:28 PDT
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.)
Comment 4 Darin Fisher (:fishd, Google) 2011-04-14 09:02:07 PDT
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.)
Comment 5 Eric U. 2011-04-14 09:24:29 PDT
Comment on attachment 89557 [details]
Patch

What about the flags TEMPORARY and PERSISTENT?  Should we prefix them too?
Comment 6 Eric U. 2011-04-14 09:25:16 PDT
(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.
Comment 7 Taiju Tsuiki 2011-04-17 19:02:10 PDT
Created attachment 89982 [details]
Patch
Comment 8 WebKit Review Bot 2011-04-17 19:24:42 PDT
Attachment 89982 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8459821
Comment 9 Build Bot 2011-04-17 19:57:37 PDT
Attachment 89982 [details] did not build on win:
Build output: http://queues.webkit.org/results/8459825
Comment 10 Early Warning System Bot 2011-04-17 20:00:56 PDT
Attachment 89982 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8435870
Comment 11 Taiju Tsuiki 2011-04-17 22:01:51 PDT
Created attachment 89992 [details]
Patch
Comment 12 WebKit Review Bot 2011-04-17 22:09:34 PDT
Attachment 89992 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8442874
Comment 13 Build Bot 2011-04-17 22:57:14 PDT
Attachment 89992 [details] did not build on win:
Build output: http://queues.webkit.org/results/8459854
Comment 14 Taiju Tsuiki 2011-04-17 22:58:58 PDT
Created attachment 89995 [details]
Patch
Comment 15 Taiju Tsuiki 2011-04-17 23:06:38 PDT
Created attachment 89996 [details]
Patch
Comment 16 Taiju Tsuiki 2011-04-18 00:20:03 PDT
Created attachment 89999 [details]
Patch
Comment 17 Kinuko Yasuda 2011-04-18 06:38:05 PDT
(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?)
Comment 18 WebKit Commit Bot 2011-04-18 22:36:53 PDT
Comment on attachment 89999 [details]
Patch

Clearing flags on attachment: 89999

Committed r84224: <http://trac.webkit.org/changeset/84224>
Comment 19 WebKit Commit Bot 2011-04-18 22:37:01 PDT
All reviewed patches have been landed.  Closing bug.