========================================================================
http://mondrian.corp.google.com/file/9228479///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc?a=1
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc 
(snapshot 1)
------------------------------------
Line 312: ModuleEnvironment *module_environment,
On 3:28 pm, noel wrote:
> make ModuleEnvironment *module_environment the first argument here to match
the
> rest of the code?

Done.
------------------------------------
Line 461: PRInt64 file_size;
On 6:05 pm, noel wrote:
> PRInt64 file_size = 0;

Done.
------------------------------------
Line 483: PRInt64 file_size;
On 6:35 pm, noel wrote:
> PRInt64 file_size = 0; and because of line 492 for example, move these three
> lines dealing with the file size down to line 495.

Done.


> If you're feeling adventurous, move the file size detection
> outside of the #if/#endif block noting that an nsILocalFile
> is publically an nsIFile, if I recall correctly, and that GetFileSize() is a
> base class method.

Yeah, I could move it down, but then there's a bit of ugliness about #if linux
then do file->foo, #else do local_file->foo.

I'll probably tidy this up a little, anyway, because I need to check that the
file size thing follows links. But that'll be in a different CL (since that
check will be on a different OS to this client).
========================================================================
http://mondrian.corp.google.com/file/9228479///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h?a=1
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.h 
(snapshot 1)
------------------------------------
Line 36: // The xxx_out parameters are presumed to be initialized to empty (or 
zero).
On 3:25 pm, noel wrote:
> you could clear them in GetDroppedFile() to make the comment assumption
> explicit.

Done.
========================================================================
http://mondrian.corp.google.com/file/9228479///depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc?a=1
File //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc 
(snapshot 1)
------------------------------------
Line 235: // aggregate file metadata) during dragenter, dragover and dragleave,
On 6:11 pm, noel wrote:
> Probably not in dragleave but it won't hurt either.

Yeah, we need to decide on what the behavior should be, and ensure that we
achieve that cross-platform.
========================================================================

-- 
To respond, reply to this email or visit http://mondrian.corp.google.com/9228479

Reply via email to