On Fri, Sep 6, 2013 at 1:40 PM, Jesse MacFadyen <[email protected]>wrote:
> Inline. > > > > >> On Sep 5, 2013, at 7:59 PM, Ian Clelland <[email protected]> > wrote: > >> > >> On Thu, Sep 5, 2013 at 8:45 PM, Jesse <[email protected]> wrote: > >> > >> I am working through some of the failing tests for wp8 filetransfer and > >> have some issues with the following tests. Can anyone add any input on > >> where some of the following requirements come from? > >> > >> 1. filetransfer.spec.7 should be able to download a file using file:// > >> (when hosted from file://) > >> > >> > >> {code} > >> if (!/^file/.exec(remoteFile)) { > >> expect(remoteFile).toMatch(/^file:/); > >> return; > >> } > >> {/code} > >> > >> > >> This will fail on wp7, wp8, and windows8, all of which are NOT loaded > from > >> the file:// protocol, but have their own specific file-like protocols. > >> ( windows phone uses: x-wmapp0:, and windows 8 uses ms-appx:// ) > >> This should not be a failure, from what I can tell. I plan to expand > the > >> test to include the current protocol, + specifically load something > using > >> the file:// protocol > > > > I suspect that if WP and Win apps cannot be hosted on file:/// URLs, then > > this test shoudn't be run on them. We should probably change it, then, to > > test that file downloads work from same-scheme / same-host origins. > > > > (I figure that this test exists for platforms like Android, where you > have > > to explicitly grant the webview access to file:/// URLs from native code, > > or else it will not allow you to access them at all, even if your app is > > also hosted at that origin.) > > > > I'm not familiar with the WP architecture -- are file:/// resources > allowed > > at all? Or are all device-local-resources accessed from the custom > schemes? > > > > File resources are not used. Assets are loaded from x-wmapp0: > I of course resolve file:// for the file API and filetransfer as well > as local xhr. > > <img src='file://kitty.jpg'> will not work > > > > > > >> > >> 2. filetransfer.spec.10 should be stopped by abort() right away > >> > >> {code} > >> expect(new Date() - startTime).toBeLessThan(300); > >> {/code} > >> > >> Where does the 300 ms rule come from? Shouldn't this just test that > aborted > >> downloads do not complete? > > > > > > The test itself is verifying that file transfers are asynchronous. If it > > fails, it means that the ft.abort() call did not get through in 300ms, > > which is a good indication that the system was blocking on the transfer. > > > > File transfers should not leave the webview unresponsive, and should > handle > > abort() calls immediately. > > I currently wait until I have interrupted the downloading thread > before dispatching the abort error. This in no way blocks the UI > thread. Based on network conditions and buffer size, this could take > longer than 300 ms. > I could change native code to dispatch aborterror immediately to meet > the 300 ms deadline, but the number is still arbitrary and does not > mean the webview is not blocked. We might as well just have the js > code for abort() dispatch the error, then it will pass everywhere. > > Agree it's a bit silly. I'm fine if you want to delete this test. > > >> and that the target file is not partially > >> written when aborted? > > > > There's another test for that: filetransfer.spec.9 checks that partial > (or > > complete) files aren't left around after an abort()ed transfer. > > > > > >> 3. filetransfer.spec.27 should be able to set custom headers > >> {code} > >> options.headers = { > >> "CustomHeader1": "CustomValue1", > >> "CustomHeader2": ["CustomValue2", "CustomValue3"], > >> }; > >> {/code} > >> > >> Is it required that we set headers to deeply nested object literals? > > > > I believe this syntax is used to set two values for the same header. This > > would appear in the request header as > > > > CustomHeader1: CustomValue1 > > CustomHeader2: CustomValue2 > > CustomHeader2: CustomValue3 > > > > It should also be allowed to output them like this: > > > > CustomHeader1: CustomValue1 > > CustomHeader2: CustomValue2, CustomValue3 > > > > I'll need to compare the outcome on other platforms then. > > > > > Is it required that we support non-well-formed object literals? hence the > >> extra ','? > > > > No, that's an error. > > > > > >> > >> 4. Android test / dependency on device > >> {code} > >> var getMalformedUrl = function() { > >> if (device.platform.match(/Android/i)) { > >> // bad protocol causes a MalformedUrlException on Android > >> return "httpssss://example.com"; > >> }... > >> {/code} > >> > >> This test is dependent on cordova-plugin-device, which may not be > present. > >> I will switch this to use a userAgent check for Android. > > > > If you can wait until tomorrow, I am planning on fixing this :) > Apparently > > there's a long-standing feature request somewhere to provide that info as > > 'cordova.platform', and now that Device is an optional plugin, it makes a > > lot more sense. I was lamenting earlier today that I had to do the same > > userAgent check in one of our plugins, so I'm just going to take care of > it. > > > > Ian > > Done yet? >
