Need to implement support for the 'mediatype' attribute on the <upload> element.

Javier suggests that http://lxr.mozilla.org/seamonkey/source/netwerk/mime/public/nsIMIMEService.idl will be a valuable interface to understand in order to implement this in case he isn't the one that eventually does the work.

blocks 1.0 2nd ed test case: 8.1.6.a this blocks bug 322255 though I don't have authority to update the bug

If the mediatype attribute is present we parse the space delimited list of mime types, map the mime types to file extensions using nsIMIMEService, and build a file extension filter string for the file open dialog. Note that the test case has a typo: image/jpg must be image/jpeg

(In reply to comment #4) > Created an attachment (id=233187) [edit] > Handle mediatype attribute > > If the mediatype attribute is present we parse the space delimited list of mime > types, map the mime types to file extensions using nsIMIMEService, and build a > file extension filter string for the file open dialog. > > Note that the test case has a typo: image/jpg must be image/jpeg > I'd propose to wait for the bug 347327 since your patch ingores mediatype element but mentioned bug adds its support. Later you can see mozType:mediatype attribute on upload element (taking into account I should do a little modification in my patch to support upload :)). What do you think?

I guess @mediatype gives a hint how to render upload control, i.e. we should have several xf:upload bindings for different types of @mediatype. Do we want to have this? If true then I guess we should post new bug since current bug is not about it. Right?

(In reply to comment #6) > I guess @mediatype gives a hint how to render upload control, i.e. we should > have several xf:upload bindings for different types of @mediatype. Do we want > to have this? If true then I guess we should post new bug since current bug is > not about it. Right? > I can't think of any examples where the control would be different based on the mime type. Do you have an idea of which ones would be different?

(In reply to comment #5) > I'd propose to wait for the bug 347327 since your patch ingores mediatype > element but mentioned bug adds its support. Later you can see mozType:mediatype > attribute on upload element (taking into account I should do a little > modification in my patch to support upload :)). What do you think? I think it could go either way. I could wait for bug 347327 to be complete and then change the two lines of code in my patch that simply gets the mediatype attribute to check for the mediatype element instead OR we can check in this bug and the change could be made as part of bug 347327. Given that this bug fixes many of the failing test cases in the test suite, I would prefer the latter.

(In reply to comment #8) > (In reply to comment #5) > > I'd propose to wait for the bug 347327 since your patch ingores mediatype > > element but mentioned bug adds its support. Later you can see mozType:mediatype > > attribute on upload element (taking into account I should do a little > > modification in my patch to support upload :)). What do you think? > > I think it could go either way. I could wait for bug 347327 to be complete and > then change the two lines of code in my patch that simply gets the mediatype > attribute to check for the mediatype element instead OR we can check in this > bug and the change could be made as part of bug 347327. Given that this bug > fixes many of the failing test cases in the test suite, I would prefer the > latter. > I'd vote that this bug wait for 347327, which is already in the review process.

(In reply to comment #7) > I can't think of any examples where the control would be different based on the > mime type. Do you have an idea of which ones would be different? > Nothing special, I'm just looking at specs (http://www.w3.org/TR/xforms/index-all.html#ui-upload). Here there is upload's picture for mediatype="image/*". I don't know do we need something like this? At least the showed upload control looks friendly I guess.

(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #5) > > > I'd propose to wait for the bug 347327 since your patch ingores mediatype > > > element but mentioned bug adds its support. Later you can see mozType:mediatype > > > attribute on upload element (taking into account I should do a little > > > modification in my patch to support upload :)). What do you think? > > > > I think it could go either way. I could wait for bug 347327 to be complete and > > then change the two lines of code in my patch that simply gets the mediatype > > attribute to check for the mediatype element instead OR we can check in this > > bug and the change could be made as part of bug 347327. Given that this bug > > fixes many of the failing test cases in the test suite, I would prefer the > > latter. > > > > I'd vote that this bug wait for 347327, which is already in the review process. > Perhaps I'm missing something, bug 347327 deals with mediatype element on output element. This bug deals with mediatype attribute on the upload element. Where is the conflict?

(In reply to comment #5) > (In reply to comment #4) > > Created an attachment (id=233187) [edit] > > Handle mediatype attribute > > > > If the mediatype attribute is present we parse the space delimited list of mime > > types, map the mime types to file extensions using nsIMIMEService, and build a > > file extension filter string for the file open dialog. > > > > Note that the test case has a typo: image/jpg must be image/jpeg > > > > I'd propose to wait for the bug 347327 since your patch ingores mediatype > element but mentioned bug adds its support. Later you can see mozType:mediatype > attribute on upload element (taking into account I should do a little > modification in my patch to support upload :)). What do you think? > Well, upload's mediatype element has been supported for sometime. This bug doesn't deal with that node. See http://lxr.mozilla.org/seamonkey/source/extensions/xforms/nsXFormsUploadElement.cpp#342

(In reply to comment #11) > Perhaps I'm missing something, bug 347327 deals with mediatype element on > output element. Not only, bug 347327 deals with @mediatype attribute too ;) > This bug deals with mediatype attribute on the upload element. > Where is the conflict? > When we deal with some mediatype issues then we should take into account attribute and element I guess since mediatype can be specified by @mediatype attribute or by mediatype element both.

The more I look at this, the more I realize that there is no overlap as far as 1.0 goes. In XForms 1.0, xf:mediatype has no affect on xf:upload (though xf:upload does affect xf:mediatype's bound value). In XForms 1.1, xf:mediatype affects the control that contains it. While in XForms 1.1 or 1.2 there might possibly be overlap (an upload without @mediatype might be able to inherit this from a contained xf:mediatype in the future), there isn't yet. So let's keep mediatype as it pertains to upload and mediatype as it pertains to output seperate. We can merge them together if a future XForms spec changes that seperation.

Comment on attachment 233187 [details] [diff] [review] Handle mediatype attribute >Index: nsXFormsUploadElement.cpp >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUploadElement.cpp,v >retrieving revision 1.18 >diff -u -8 -p -r1.18 nsXFormsUploadElement.cpp >--- nsXFormsUploadElement.cpp 10 Aug 2006 21:41:25 -0000 1.18 >+++ nsXFormsUploadElement.cpp 11 Aug 2006 00:34:15 -0000 >@@ -202,19 +202,65 @@ nsXFormsUploadElement::PickFile() > nsCOMPtr<nsIFilePicker> filePicker = > do_CreateInstance("@mozilla.org/filepicker;1"); > if (!filePicker) > return NS_ERROR_FAILURE; > > rv = filePicker->Init(internal, filepickerTitle, nsIFilePicker::modeOpen); > NS_ENSURE_SUCCESS(rv, rv); > >- // set filter "All Files" >- // XXX implement "@mediatype" file filters >- filePicker->AppendFilters(nsIFilePicker::filterAll); >+ // Set the file picker filters based on the mediatype attribute. >+ nsAutoString mediaType; >+ mElement->GetAttribute(NS_LITERAL_STRING("mediatype"), mediaType); >+ >+ if (mediaType.IsEmpty()) { >+ filePicker->AppendFilters(nsIFilePicker::filterAll); >+ } >+ else { nit: else goes on line above to keep style consistent >+ // The mediatype attribute contains a space delimited list of mime types. >+ nsresult rv; >+ nsCOMPtr<nsIMIMEService> mimeService = >+ do_GetService("@mozilla.org/mime;1", &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ nsAString::const_iterator start, end, iter; >+ mediaType.BeginReading(start); >+ mediaType.BeginReading(iter); >+ mediaType.EndReading(end); >+ >+ nsAutoString fileFilter; >+ nsAutoString mimeType; >+ while (iter != end) { >+ if (FindCharInReadable(' ', iter, end)) { >+ mimeType = Substring(start, iter); >+ // Skip the space. >+ ++iter; >+ // Save the starting position for the next mime type (if any). >+ start = iter; >+ } >+ else { >+ // Only 1 media type or we've reached the end of the list. >+ mimeType = Substring(start, end); >+ } >+ >+ // Map the mime type to a file extension and add the extension to the file >+ // type filter for the file dialog. >+ nsCAutoString fileExtension; >+ rv = mimeService->GetPrimaryExtension(NS_ConvertUTF16toUTF8(mimeType), >+ EmptyCString(), fileExtension); >+ if (NS_SUCCEEDED(rv) && !fileExtension.IsEmpty()) { >+ fileFilter.AppendLiteral("*."); >+ fileFilter.Append(NS_ConvertUTF8toUTF16(fileExtension)); >+ fileFilter.AppendLiteral(";"); >+ } >+ } >+ >+ // Append the file extension filter. >+ filePicker->AppendFilter(fileFilter, fileFilter); >+ } I'm thinking maybe the file filter should be 'all files' if we don't find any mime types. The file picker shows all the files anyhow and it just looks weird if the "files of type" field is blank. With those, r=me

Always add 'All Files' as a filter. Do not append the semicolon to the last filter in the list.

corrected testcase per merle's suggestion

checked into trunk for msterlin

checked into 1.8.0 branch on 2006/09/21

checked into 1.8 branch on 2006/11/21