Bug 35361 – File.type support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Jian Li]

Need to implement File.type as defined in File API spec.

[Jian Li]

Created attachment 49443 [details]
Proposed Patch

[Dmitry Titov]

Is the non-existing file detected as "application/octet-stream" on all platforms? The test update seems to assume this.

[Jian Li]

Created attachment 49450 [details]
Proposed Patch

Changed to call the function that returns empty string on failure, in order to stick to the spec.

For non-existent file, we do not perform the check since the check will involve synchronous file operation and the file might become valid right before accessing it.

[Dmitry Titov]

Comment on attachment 49450 [details]
Proposed Patch

> diff --git a/WebCore/html/File.cpp b/WebCore/html/File.cpp
>  File::File(const String& path)
>      : Blob(path)
>      , m_name(pathGetFileName(path))
> +    // We don't use MIMETypeRegistry::getMIMETypeForPath() because it returns "application/octet-stream" upon failure
> +    , m_type(MIMETypeRegistry::getMIMETypeForExtension(path.substring(path.reverseFind('.') + 1)))

It looks too complex code to have it in initializer. Also, for the path "gif" it will return a valid mime type while it should return "".
Could it be moved into ctor body with more checks?

Also, getMIMETypeForExtension on some platforms also returns non-empty string when it can't match the extension - see for example MIMETypeRegistryQt.cpp
It might be a good idea to ask port developers if they want to fix those.

r- because of initializer.

[Jian Li]

Created attachment 49514 [details]
Proposed Patch

[Jian Li]

(In reply to comment #4)
> (From update of attachment 49450 [details])
> > diff --git a/WebCore/html/File.cpp b/WebCore/html/File.cpp
> >  File::File(const String& path)
> >      : Blob(path)
> >      , m_name(pathGetFileName(path))
> > +    // We don't use MIMETypeRegistry::getMIMETypeForPath() because it returns "application/octet-stream" upon failure
> > +    , m_type(MIMETypeRegistry::getMIMETypeForExtension(path.substring(path.reverseFind('.') + 1)))
> 
> It looks too complex code to have it in initializer. Also, for the path "gif"
> it will return a valid mime type while it should return "".
> Could it be moved into ctor body with more checks?

Indeed the above code handles the file with name "gif" since we're finding in the full path.

Moved to the body and changed to use the name to make the logic clear.
> 
> Also, getMIMETypeForExtension on some platforms also returns non-empty string
> when it can't match the extension - see for example MIMETypeRegistryQt.cpp
> It might be a good idea to ask port developers if they want to fix those.
> 
I will ping qt developer on this.

> r- because of initializer.

[Dmitry Titov]

Comment on attachment 49514 [details]
Proposed Patch

r=me

> diff --git a/WebCore/html/File.cpp b/WebCore/html/File.cpp
> +    // We don't use MIMETypeRegistry::getMIMETypeForPath() because it returns "application/octet-stream" upon failure

Nit - please add a "." at the end of this sentence.

[Jian Li]

Committed as http://trac.webkit.org/changeset/55257.