======================================================================== http://mondrian.corp.google.com/file/10892071///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h?a=1 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h (snapshot 1) ------------------------------------ Line 52: DRAG_AND_DROP_EVENT_DROP, On 4:52 pm, 29 Apr, nigeltao wrote: > Strip the trailing comma.
Done. ======================================================================== http://mondrian.corp.google.com/file/10892071///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc?a=5 File //depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_cr.cc (snapshot 5) ------------------------------------ Line 241: // "Say" there's 1 drag file until we know more; extract the files. On 11:34 am, nigeltao wrote: > Do we really want 1 here, or is 0 more appropriate, in case we early exit (e.g. > if drag->GetDragData below fails)? when we early exit or fail -- imagine the user is dragging 3 directories -- then the call site in GetDragData() clears the file meta data file cache, but leaves g_drag_files alone. g_drag_files says there are files. So the meta data we return is non-NULL and only has the "count" member with a count = 0. So there are files, just none we can support, and we report that to the user javascript app, so it can decide to set drag cursor to "none". So why suggest that 0 more appropriate? In that case, we'd set g_drag_files to 0, and return NULL to user javascript. Best that javascript can do is say "ok, there are no files" (when in fact there are), and decide "well, maybe there's a URL or Text, let's try those". You have tested that case, right? What does event.dataTransfer.getData("Text"|"URL") return if we return NULL when files are being dragged? ------------------------------------ Line 331: if (drag.BrowserSupportsDragDrop()) On 11:31 am, nigeltao wrote: > I feel better, with nested if's, when they all have {} curly brackets, even if > they are one-liners. But also, should the drag.BrowserSupportsDragDrop check > happen before you call drag.SetDropEffect? Is what you really want something > like: > if (!drag.BSDD || !d.SDE) { > *error = ... > } added the curlys here and at @331 below. I ordered the calls for the expected case -- the browser does support drag drop drag -- and SetDropEffect() checks too, and so does GetDragType() @331. If you reverse the order, it still works, but the cost is few more lines of code (meh) but a loss of repetition at the invocation points. I prefer to repeat a theme, because in my experience if you reintroduce a prior theme in code, but in slightly different way, you tend to confuse the reader. ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/10892071
