You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Simon MacDonald <si...@gmail.com> on 2012/02/24 20:10:22 UTC
Bug in FileSystem.js
Becky has found a bug in FileSystem.js and I just wanted to bring it up
on the list before making any changes. At this line:
https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
you see a new DirectoryEntry is created using the *name* of the filesystem
and the *root* path of the filesystem. What is being passed in is something
like this:
name = PERSISTENT
root = /data/data/com.phonegap
so when the DirectoryEntry is created it will have a name of
*PERSISTENT*which is wrong. The name property should be the name of
the file or
*com.phonegap* in my example.
Does this makes sense to everyone or are we going crazy?
Simon Mac Donald
http://hi.im/simonmacdonald
Re: Bug in FileSystem.js
Posted by Simon MacDonald <si...@gmail.com>.
I think we should make the change that Becky describe in point 1 of her
earlier email. It will need to be done in common-js then across the
platforms.
Simon Mac Donald
http://hi.im/simonmacdonald
On Fri, Feb 24, 2012 at 6:50 PM, Joe Bowser <bo...@gmail.com> wrote:
> Hey
>
> I'm looking at this, and the more I think about it, the more I'm thinking
> that the name actually should be null if we're getting the directory
> entries of the Persistant and Temporary filesystems? I'm currently looking
> over that code again, and I'm not sure what should be correct. Any
> thoughts?
>
> Joe
>
> On Fri, Feb 24, 2012 at 1:42 PM, Joe Bowser <bo...@gmail.com> wrote:
>
> > They shouldn't be able to read that directory anyway unless they rooted
> > their own phone. We just need to be able to handle the error that it
> > throws, I think.
> >
> > Joe
> >
> >
> > On Fri, Feb 24, 2012 at 12:21 PM, Simon MacDonald <
> > simon.macdonald@gmail.com> wrote:
> >
> >> So then the root directory of the persistent filesystem should look
> like:
> >>
> >> { name: "", fullPath: "/data/data/com.phonegap" }
> >>
> >> then if I create a cache directory off the root it would be:
> >>
> >> { name: "cache", fullPath: "/data/data/com.phonegap/cache" }
> >>
> >> but what do we do about someone who goes back two directories? You would
> >> end up at:
> >>
> >> { name: "data", fullPath: "/data" }
> >>
> >> Do we need to prevent that type of behaviour?
> >>
> >> Simon Mac Donald
> >> http://hi.im/simonmacdonald
> >>
> >>
> >> On Fri, Feb 24, 2012 at 2:33 PM, Joe Bowser <bo...@gmail.com> wrote:
> >>
> >> > No, I think you are partially right. I think the name should be blank
> >> for
> >> > the root directory of the persistent storage. It definitely shouldn't
> >> say
> >> > that it's persistent, that seems to be very wrong.
> >> >
> >> > Joe
> >> >
> >> > On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <
> >> > simon.macdonald@gmail.com
> >> > > wrote:
> >> >
> >> > > Becky has found a bug in FileSystem.js and I just wanted to bring it
> >> up
> >> > > on the list before making any changes. At this line:
> >> > >
> >> > >
> >> > >
> >> >
> >>
> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
> >> > >
> >> > > you see a new DirectoryEntry is created using the *name* of the
> >> > filesystem
> >> > > and the *root* path of the filesystem. What is being passed in is
> >> > something
> >> > > like this:
> >> > >
> >> > > name = PERSISTENT
> >> > > root = /data/data/com.phonegap
> >> > >
> >> > > so when the DirectoryEntry is created it will have a name of
> >> > > *PERSISTENT*which is wrong. The name property should be the name of
> >> > > the file or
> >> > > *com.phonegap* in my example.
> >> > >
> >> > > Does this makes sense to everyone or are we going crazy?
> >> > >
> >> > > Simon Mac Donald
> >> > > http://hi.im/simonmacdonald
> >> > >
> >> >
> >>
> >
> >
>
Re: Bug in FileSystem.js
Posted by Joe Bowser <bo...@gmail.com>.
Hey
I'm looking at this, and the more I think about it, the more I'm thinking
that the name actually should be null if we're getting the directory
entries of the Persistant and Temporary filesystems? I'm currently looking
over that code again, and I'm not sure what should be correct. Any thoughts?
Joe
On Fri, Feb 24, 2012 at 1:42 PM, Joe Bowser <bo...@gmail.com> wrote:
> They shouldn't be able to read that directory anyway unless they rooted
> their own phone. We just need to be able to handle the error that it
> throws, I think.
>
> Joe
>
>
> On Fri, Feb 24, 2012 at 12:21 PM, Simon MacDonald <
> simon.macdonald@gmail.com> wrote:
>
>> So then the root directory of the persistent filesystem should look like:
>>
>> { name: "", fullPath: "/data/data/com.phonegap" }
>>
>> then if I create a cache directory off the root it would be:
>>
>> { name: "cache", fullPath: "/data/data/com.phonegap/cache" }
>>
>> but what do we do about someone who goes back two directories? You would
>> end up at:
>>
>> { name: "data", fullPath: "/data" }
>>
>> Do we need to prevent that type of behaviour?
>>
>> Simon Mac Donald
>> http://hi.im/simonmacdonald
>>
>>
>> On Fri, Feb 24, 2012 at 2:33 PM, Joe Bowser <bo...@gmail.com> wrote:
>>
>> > No, I think you are partially right. I think the name should be blank
>> for
>> > the root directory of the persistent storage. It definitely shouldn't
>> say
>> > that it's persistent, that seems to be very wrong.
>> >
>> > Joe
>> >
>> > On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <
>> > simon.macdonald@gmail.com
>> > > wrote:
>> >
>> > > Becky has found a bug in FileSystem.js and I just wanted to bring it
>> up
>> > > on the list before making any changes. At this line:
>> > >
>> > >
>> > >
>> >
>> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
>> > >
>> > > you see a new DirectoryEntry is created using the *name* of the
>> > filesystem
>> > > and the *root* path of the filesystem. What is being passed in is
>> > something
>> > > like this:
>> > >
>> > > name = PERSISTENT
>> > > root = /data/data/com.phonegap
>> > >
>> > > so when the DirectoryEntry is created it will have a name of
>> > > *PERSISTENT*which is wrong. The name property should be the name of
>> > > the file or
>> > > *com.phonegap* in my example.
>> > >
>> > > Does this makes sense to everyone or are we going crazy?
>> > >
>> > > Simon Mac Donald
>> > > http://hi.im/simonmacdonald
>> > >
>> >
>>
>
>
Re: Bug in FileSystem.js
Posted by Joe Bowser <bo...@gmail.com>.
They shouldn't be able to read that directory anyway unless they rooted
their own phone. We just need to be able to handle the error that it
throws, I think.
Joe
On Fri, Feb 24, 2012 at 12:21 PM, Simon MacDonald <simon.macdonald@gmail.com
> wrote:
> So then the root directory of the persistent filesystem should look like:
>
> { name: "", fullPath: "/data/data/com.phonegap" }
>
> then if I create a cache directory off the root it would be:
>
> { name: "cache", fullPath: "/data/data/com.phonegap/cache" }
>
> but what do we do about someone who goes back two directories? You would
> end up at:
>
> { name: "data", fullPath: "/data" }
>
> Do we need to prevent that type of behaviour?
>
> Simon Mac Donald
> http://hi.im/simonmacdonald
>
>
> On Fri, Feb 24, 2012 at 2:33 PM, Joe Bowser <bo...@gmail.com> wrote:
>
> > No, I think you are partially right. I think the name should be blank
> for
> > the root directory of the persistent storage. It definitely shouldn't
> say
> > that it's persistent, that seems to be very wrong.
> >
> > Joe
> >
> > On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <
> > simon.macdonald@gmail.com
> > > wrote:
> >
> > > Becky has found a bug in FileSystem.js and I just wanted to bring it up
> > > on the list before making any changes. At this line:
> > >
> > >
> > >
> >
> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
> > >
> > > you see a new DirectoryEntry is created using the *name* of the
> > filesystem
> > > and the *root* path of the filesystem. What is being passed in is
> > something
> > > like this:
> > >
> > > name = PERSISTENT
> > > root = /data/data/com.phonegap
> > >
> > > so when the DirectoryEntry is created it will have a name of
> > > *PERSISTENT*which is wrong. The name property should be the name of
> > > the file or
> > > *com.phonegap* in my example.
> > >
> > > Does this makes sense to everyone or are we going crazy?
> > >
> > > Simon Mac Donald
> > > http://hi.im/simonmacdonald
> > >
> >
>
Re: Bug in FileSystem.js
Posted by Filip Maj <fi...@adobe.com>.
>Making this change gets me further in the iOS update. However, I don't
>understand why the other platforms are returning a URL in the fullPath
>parameter. Will start/continue a thread on that.
>
>-becky
This is left-over baggage from previous implementations. Figuring out a
good way to abstract would be nice.
Basically: each platform implements file system paths/URIs differently.
The W3C spec recommends an even different way. The holy grail would be
File API code that we could literally copy-paste across platforms and see
it "just work."
What I've seen employed and what I've seen recommended by various people:
- an app: URI. app:/ or app:// or app:/// would be the root of your app
"jail" - leaving it up to the implementations to figure out how to do
that, decoding the paths on the native side, etc. The W3C widget spec I
believe has some similar suggestions with respect to using a widget:// URI.
- using file:///. Then you get an absolute path to the device. Encompasses
cases where you want to mess about the FS outside of your app and globally
on the device. Maybe we could use some kind of convention here but it
could get messy.
- just using absolute or relative URLs, regardless of protocol. Like "/"
or "/cache/".
- I think the recommendation in the latest File API W3C spec draft has
something like http://example.domain/app/ - but this doesn't make much
sense to me.
In all honesty, I have little preference to either of these. As long as it
is consistent :)
For 1.5, let's leave the current baggage and let it "just work." Moving
forward, let's document and discuss this, let our community know, and fix
it.
Re: Bug in FileSystem.js
Posted by Becky Gibson <gi...@gmail.com>.
Regading the FileSystem.name entry. The W3C spec is somewhat ambiguous on
this so we implemented "persistent" and "temporary" when we did the
original BB, iOS and Android implementations. I would prefer we debate
changing that later.
Regarding the DirectoryEntry name parameter, the spec indicates that is
just the name of the directory, excluding the path leading to it. This is
how BB, iOS and Android initially implemented this. We can solve the
DirectoryEntry creation but with the following two changes:
1) modify the FileSystem constructor in FileSystem.js to the following:
var FileSystem = function(name, root) {
this.name = name || null;
if (root) {
this.root = new DirectoryEntry(root.name, root.fullPath);
}
};
2) In the device code when returning from requestFileSystem the device code
should return a pseudo FileSystem object:
.name is the name of the file system (currently "persistent" or
"temporary") and .root is a DirectoryEntry pseudo object (it has allof the
paramters but there are no functions defined which is why we have to go
through the full object creation). This is what BB, iOS and Android
originally did, but I believe BB and Android had changed this to return the
fullPath in the .root parameter.
Making this change gets me further in the iOS update. However, I don't
understand why the other platforms are returning a URL in the fullPath
parameter. Will start/continue a thread on that.
-becky
-becky
On Fri, Feb 24, 2012 at 3:21 PM, Simon MacDonald
<si...@gmail.com>wrote:
> So then the root directory of the persistent filesystem should look like:
>
> { name: "", fullPath: "/data/data/com.phonegap" }
>
> then if I create a cache directory off the root it would be:
>
> { name: "cache", fullPath: "/data/data/com.phonegap/cache" }
>
> but what do we do about someone who goes back two directories? You would
> end up at:
>
> { name: "data", fullPath: "/data" }
>
> Do we need to prevent that type of behaviour?
>
> Simon Mac Donald
> http://hi.im/simonmacdonald
>
>
> On Fri, Feb 24, 2012 at 2:33 PM, Joe Bowser <bo...@gmail.com> wrote:
>
> > No, I think you are partially right. I think the name should be blank
> for
> > the root directory of the persistent storage. It definitely shouldn't
> say
> > that it's persistent, that seems to be very wrong.
> >
> > Joe
> >
> > On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <
> > simon.macdonald@gmail.com
> > > wrote:
> >
> > > Becky has found a bug in FileSystem.js and I just wanted to bring it up
> > > on the list before making any changes. At this line:
> > >
> > >
> > >
> >
> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
> > >
> > > you see a new DirectoryEntry is created using the *name* of the
> > filesystem
> > > and the *root* path of the filesystem. What is being passed in is
> > something
> > > like this:
> > >
> > > name = PERSISTENT
> > > root = /data/data/com.phonegap
> > >
> > > so when the DirectoryEntry is created it will have a name of
> > > *PERSISTENT*which is wrong. The name property should be the name of
> > > the file or
> > > *com.phonegap* in my example.
> > >
> > > Does this makes sense to everyone or are we going crazy?
> > >
> > > Simon Mac Donald
> > > http://hi.im/simonmacdonald
> > >
> >
>
Re: Bug in FileSystem.js
Posted by Simon MacDonald <si...@gmail.com>.
So then the root directory of the persistent filesystem should look like:
{ name: "", fullPath: "/data/data/com.phonegap" }
then if I create a cache directory off the root it would be:
{ name: "cache", fullPath: "/data/data/com.phonegap/cache" }
but what do we do about someone who goes back two directories? You would
end up at:
{ name: "data", fullPath: "/data" }
Do we need to prevent that type of behaviour?
Simon Mac Donald
http://hi.im/simonmacdonald
On Fri, Feb 24, 2012 at 2:33 PM, Joe Bowser <bo...@gmail.com> wrote:
> No, I think you are partially right. I think the name should be blank for
> the root directory of the persistent storage. It definitely shouldn't say
> that it's persistent, that seems to be very wrong.
>
> Joe
>
> On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <
> simon.macdonald@gmail.com
> > wrote:
>
> > Becky has found a bug in FileSystem.js and I just wanted to bring it up
> > on the list before making any changes. At this line:
> >
> >
> >
> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
> >
> > you see a new DirectoryEntry is created using the *name* of the
> filesystem
> > and the *root* path of the filesystem. What is being passed in is
> something
> > like this:
> >
> > name = PERSISTENT
> > root = /data/data/com.phonegap
> >
> > so when the DirectoryEntry is created it will have a name of
> > *PERSISTENT*which is wrong. The name property should be the name of
> > the file or
> > *com.phonegap* in my example.
> >
> > Does this makes sense to everyone or are we going crazy?
> >
> > Simon Mac Donald
> > http://hi.im/simonmacdonald
> >
>
Re: Bug in FileSystem.js
Posted by Joe Bowser <bo...@gmail.com>.
No, I think you are partially right. I think the name should be blank for
the root directory of the persistent storage. It definitely shouldn't say
that it's persistent, that seems to be very wrong.
Joe
On Fri, Feb 24, 2012 at 11:10 AM, Simon MacDonald <simon.macdonald@gmail.com
> wrote:
> Becky has found a bug in FileSystem.js and I just wanted to bring it up
> on the list before making any changes. At this line:
>
>
> https://github.com/apache/incubator-cordova-js/blob/master/lib/plugin/FileSystem.js#L13
>
> you see a new DirectoryEntry is created using the *name* of the filesystem
> and the *root* path of the filesystem. What is being passed in is something
> like this:
>
> name = PERSISTENT
> root = /data/data/com.phonegap
>
> so when the DirectoryEntry is created it will have a name of
> *PERSISTENT*which is wrong. The name property should be the name of
> the file or
> *com.phonegap* in my example.
>
> Does this makes sense to everyone or are we going crazy?
>
> Simon Mac Donald
> http://hi.im/simonmacdonald
>