GNOME Bugzilla – Bug 171180
Dragging to address field should replace current contents
Last modified: 2013-12-16 16:58:27 UTC
Steps to reproduce: 1. Drag an URL to the address field. What should happen: * The URL you are dragging replaces the current contents of the address field. What actually happens: * http://foo.examhttp://bar.example.net/thisisnotwhatiwant.htmlple.com/ Other information:
I do agree but I'm afraid it's something with the way GTK handles DnD text.
Can this problem be addressed in GTK? If not, I suggest to mark this WONTFIX.
With respect, that doesn't make any sense. Whether it's desirable behavior for a Web browser is independent of what that browser is made of.
to fix this, the location entry must accept drops of urls and handle them differently from mere drops of text.
OK,updating version fields then.
Here is a patch for this. With the patch, dropping text and URL:s to the address field works like this: 1. Select some text and drag it to the address field. 2. The contents of the address field is replaced with the selected text. 1. Select an URL an drag it to the address field. 2. The contents of the address field is replaced with the selected URL and that URL is loaded. This copies how Firefox handles selected text and URL:s. I think replacing the address fields text is much saner than inserting or appending the selected text or URL. I've added a function loc_entry_drag_data_received_cb() to ephy-location-action.c that works almost exactly like notebook_drag_data_received_cb() in ephy-notebook.c. In fact, the two handlers are so similar that it should be easy to merge the both code paths. I wasn't sure were the "setup" code for the signal handler should be placed, so I added it to the connect_proxy() and ephy_location_entry_construct_contents() functions. It probably needs to be done better.
Created attachment 76766 [details] [review] Make dropping of URL:s and text in Epiphany's location entry better
Thanks for the patch. Does selecting text (not an URL link) in the web page and dragging it to the location entry still work with the patch?
Yes, kind of. Current Epiphany behaviour is if the location entry contains "www.google.com" and you drag the text "foobar" and you place it after "www." the location entry text becomes "www.foobargoogle.com". With the patch, it works like Firefox -- "foobar" will replace "www.google.com".
I'm undecided on this; while it's fine to replace when an URL is dragged, I'm not sure it's right when just text is dragged...
Right, I can change that. It is the dragging of URL:s that are the really broken thing. The use case for replacing the text in the location bar is if you want to search for it. For example, you could drag say "undefined reference to `ldap_err2string'" to the location bar, click Go to google-search for it. On the other hand, the URL could be "http://en.wikipedia.org" and you want the dragged text "/wiki/Epiphany" to be appended to that so that you can visit that wiki page.
Yes, I think it's better to only do this for URI drags.
Agree with chpe, replacing the location entry content should happen only if the dragged data is a URI.
Created attachment 77782 [details] [review] Handle URL:s dropped in the location entry, attempt 2 Alright, I've updated the patch per your request. Dragging text should work as it did before now. I had to change entry_drag_drop_cb so that it always stops signal emission. I have no idea why that was needed.. but it works. :)
> Dragging text should work as it did before now. I had to change > entry_drag_drop_cb so that it always stops > signal emission. I have no idea why that was needed.. but it works. :) Hmm that makes me a bit nervous... :) + gtk_drag_dest_set (priv->icon_entry->entry, + GTK_DEST_DEFAULT_MOTION | GTK_DEST_DEFAULT_DROP, + url_drag_types, + G_N_ELEMENTS (url_drag_types), + GDK_ACTION_MOVE | GDK_ACTION_COPY); Shouldn't this use gtk_drag_dest_get_target_list to get the stock list (containing the text targets) and append (or prepend??) the url_drag_types, then use gtk_drag_dest_set_target_list () (and unref the list[?]) ?
Marking needs-work, see comment 15.
updating version
I'm willing to work on it, will do soon (you know I'm new to Epiphany, but I'm even newer to D'n'D).
Still an issue.
Created attachment 264159 [details] [review] Make text and URLs dropped in location entry clear entry and load new URL
Review of attachment 264159 [details] [review]: ::: src/ephy-location-controller.c @@ +166,3 @@ + + /* URL_TYPE has format: url \n title */ + split = g_strsplit ((const gchar *)sel_data, "\n", 2); Use g_uri_list_extract_uris() instead? @@ +168,3 @@ + split = g_strsplit ((const gchar *)sel_data, "\n", 2); + if (split != NULL && split[0] != NULL && + split[0][0] != '\0') If you used g_uri_list_extract_uris() I think this code would get removed, but *split[0] is more readable than split[0][0].
Created attachment 264163 [details] [review] Make text and URLs dropped in location entry clear entry and load new URL
Review of attachment 264163 [details] [review]: LGTM ::: src/ephy-location-controller.c @@ +177,3 @@ + } else if (gtk_selection_data_get_target (selection_data) == text_type) { + gtk_entry_set_text (entry, (const gchar *)sel_data); + ephy_link_open (EPHY_LINK (controller), Hmm, shouldn't we use the default behaviour for non-URL text drops? I tend to think dragging text in a URL field is mostly a mistake unless you drag a full URL, in any case.
Since the URL field is also the search field I thought it made sense to allow text drops to a) load if they are in URL form b) initiate a search if they are not.