On Thu, Sep 5, 2013 at 10:58 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? > I think you'd be fine to make this test whatever scheme you're hosted on. This behaviour is functionally the same as using resolveLocalFIleSystemURI() + Entry.copy(), but has the advantage that it provides progress callbacks. > > > > > > 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. > Right on. I added this to ensure the threading was working right on Android / iOS. On Android I actually fire the abort() callback as soon as the abort is queued to achieve this performance. > > > > 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 > > 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 >
