Fix for FLUID-1947

[Eli Cochran]

Mr. Clark,
Your changes look good. Excellent, in fact.

In thinking about that final afterFileComplete event, I realized that  
there is one more minor little problem that can wait until after 0.6.  
At the moment when the file errors, there is a call which hides the  
total progressor and updates the Total status text. Unfortunately it  
does a quick animation to 100% before it hides it even if the upload  
was cancelled.

Thanks for the review and the improvements. Since yours is the last  
patch, please commit. Although I suppose you should ask the king first.

- Eli



On Dec 13, 2008, at 9:30 AM, Colin Clark wrote:

> 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
>

. . . . . . . . . . .  .  .   .    .      .         .              .                     .

Eli Cochran
user interaction developer
ETS, UC Berkeley