Fix for FLUID-1947

[Colin Clark]

Eli,

Wow, this is incredible detective work. Thanks very much for your  
clear overview of the issue.

I gave your patch a detailed review, and it does indeed fix the issue.  
I found one thing that I think might be an error, and I made a few  
minor changes to improve readability. I've attached a new version of  
the patch to the FLUID-1947 issue:

http://issues.fluidproject.org/browse/FLUID-1947

So the one issue that I think might be an error is the removal of the  
call to the afterFileComplete event in stopDemo(). I believe we should  
still be firing an afterFileComplete event even if there was an error  
or the upload was stopped. In part, this will ensure that  
SWFUploadManager cleans up the state of the queue correctly. I put the  
call back in and tested, and it seems consistent with the behaviour of  
the server-backed version. Does this make sense, or am I missing  
something subtle here?

Here's a summary of the other changes I made, all of which were minor  
readability tweaks:

1. I renamed pauseDemo to stopDemo to be consistent with the our  
naming conventions throughout the component.
3. I removed a unnecessary nested if statement in simulateUpload() by  
first checking if we're not uploading and bailing immediately.
4. I similarly removed a layer of indenting in finishUploading() using  
the same strategy.

Can you take a look at this version of the patch and double-check that  
it is correct? If so, go ahead and commit.

Colin

On 12-Dec-08, at 8:33 PM, Eli Cochran wrote:

> It took me a long time to get my head around FLUID-1947 but finally  
> I figured out that what was happening was because we insert a delay  
> between each file progress event. We do this to simulate what would  
> happen during and actual upload, and give the user a chance to  
> respond to the behavior of the component in a simulated upload.
>
> What was happening was that between the moment that we queued up the  
> next progress and the time that the progress actually happened, the  
> user could click the Stop Upload button thus firing a bunch of other  
> events. Depending on the timing of the click, different odd things  
> would happen.
>
> So instead of doing:
>
> 	check if we can progress
> 		set next progress on the timer
> 			timer fires next progress
> 				next progress
> 				
>
> We need to
>
> 	check if we can progress
> 		set next progress on the timer
> 			timer fires next progress
> 				check if we can progress
> 					next progress
>
> I also removed the delay on finishUploading because this was another  
> place where the user could slip an event in, and it wasn't really  
> necessary for the simulation.  At the point that the finishUpload  
> fires, we should not wait but start in immediately into the next file.
>
> And I switched the code to use that.queue.isUploading instead of  
> that.demoState.shouldPause which meant that pauseDemo could be  
> simplified a little bit and we're using a consistent variable to  
> checking the state of the upload.
>
> Obviously this needs a very detailed review.
>
> - Eli
>
>
> <FLUID-1947.b.patch>
>
> . . . . . . . . . . .  .  .   .    .      .         .              .                     .
>
> Eli Cochran
> user interaction developer
> ETS, UC Berkeley
>
>

---
Colin Clark
Technical Lead, Fluid Project
Adaptive Technology Resource Centre, University of Toronto
http://fluidproject.org