GTK: Implement omnibox DnD destination behavior.

GTK: Implement omnibox DnD destination behavior.

BUG=18025
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22102

[tony, 2009-07-30 01:02:30]

http://codereview.chromium.org/160369/diff/1/2
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/160369/diff/1/2#newcode196
Line 196: //  gtk_drag_dest_set(text_view_, GTK_DEST_DEFAULT_ALL, NULL, 0,
GDK_ACTION_COPY);
Nit: 80 cols

http://codereview.chromium.org/160369/diff/1/2#newcode694
Line 694: std::wstring possible_url = UTF8ToWide(reinterpret_cast<char*>(text));
The windows code has a call to CollapseWhitespace.  Should we have one as well?

http://codereview.chromium.org/160369/diff/1/2#newcode699
Line 699: g_signal_stop_emission_by_name(text_view_, "drag-data-received");
What's this for?  Do we need to re-enable this at some point?

http://codereview.chromium.org/160369/diff/1/3
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/160369/diff/1/3#newcode235
Line 235: GtkSelectionData* selection_data, guint target_type,
Nit: 80 cols

[Daniel Erat, 2009-07-30 01:21:26]

http://codereview.chromium.org/160369/diff/1/2
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/160369/diff/1/2#newcode198
Line 198: //  GtkDndUtil::SetDestTargetList(text_view_, target_codes);
is this still in-progress?  why's there commented-out code here?

[Evan Stade, 2009-07-30 01:24:48]

http://codereview.chromium.org/160369/diff/1/2
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/160369/diff/1/2#newcode196
Line 196: //  gtk_drag_dest_set(text_view_, GTK_DEST_DEFAULT_ALL, NULL, 0,
GDK_ACTION_COPY);
On 2009/07/30 01:02:30, tony wrote:
> Nit: 80 cols

oops... all this is supposed to not be here

http://codereview.chromium.org/160369/diff/1/2#newcode694
Line 694: std::wstring possible_url = UTF8ToWide(reinterpret_cast<char*>(text));
On 2009/07/30 01:02:30, tony wrote:
> The windows code has a call to CollapseWhitespace.  Should we have one as
well?

sounds reasonable

http://codereview.chromium.org/160369/diff/1/2#newcode699
Line 699: g_signal_stop_emission_by_name(text_view_, "drag-data-received");
On 2009/07/30 01:02:30, tony wrote:
> What's this for?  Do we need to re-enable this at some point?

It's basically the same as returning TRUE for signals that have a gboolean
return value. I have no clue why certain signals force you to do it this way
instead. See elsewhere in this file for precedence.

[Evan Stade, 2009-07-30 01:24:58]

updated

[Daniel Erat, 2009-07-30 18:17:26]

LGTM

[Dean McNamee, 2009-07-30 18:21:04]

http://codereview.chromium.org/160369/diff/17/19
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/160369/diff/17/19#newcode228
Line 228: guint target_type, guint time,
guint time on its own line

[Evan Stade, 2009-07-30 18:56:57]

http://codereview.chromium.org/160369/diff/17/19
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/160369/diff/17/19#newcode228
Line 228: guint target_type, guint time,
On 2009/07/30 18:21:05, Dean McNamee wrote:
> guint time on its own line

Done.

[tony, 2009-07-30 19:09:17]

LGTM2