GNOME Bugzilla – Bug 732713
Download handled link remains in the URL bar instead of the original one
Last modified: 2014-09-17 14:08:22 UTC
This is related to bug 732679 Web 3.12.1 and WebKit 2.4.3 Go to a page with content which is handled as a download upon clicking. Click on the link, the download starts automatically, the URL in the bar is updated to the one of the downloading file. Click in the bar to edit the visiting URL, the URL is the original one from the visiting page. Click outside of the editable URL bar, the URL is the one from the downloaded content. For example, got to bug 731298 and try with the attachment.
Created attachment 284696 [details] [review] Let the location controller control the title box address By binding the title box's address directly to the URI property of the web view, we bypass all our custom logic for displaying the URI, sometimes leading to desyncs between the URI shown by the location entry and the URI shown by the title box. Also, with this change the title of the page now changes at the same time the displayed address does, whereas previously we would change the address on the start of load and the title on completion, which looked odd. Fair warning: this patch will result in incorrect URIs being displayed unless the patches in bug #734952 are merged first.
Review of attachment 284696 [details] [review]: ::: src/ephy-location-controller.c @@ +693,3 @@ + * + * The #EphyLocationController sets the address of this title box. + */ This looks bad indented. ::: src/ephy-title-box.c @@ +685,3 @@ + char *uri; + + g_return_if_fail (EPHY_IS_TITLE_BOX (title_box)); do we allow a NULL address? @@ +691,3 @@ + + if (mode == EPHY_EMBED_SHELL_MODE_APPLICATION) { + uri = g_strdup (address); I think we can avoid this allocation now, with something like: if (mode == EPHY_EMBED_SHELL_MODE_APPLICATION) { gtk_label_set_text (GTK_LABEL (priv->uri), address); } else { gboolean rtl; char *uri; .... gtk_label_set_text (GTK_LABEL (priv->uri), uri); g_free (uri); } @@ +694,3 @@ + } else { + gboolean rtl; + rtl = gtk_widget_get_default_direction () == GTK_TEXT_DIR_RTL; I know it's just copy paste, but leave a space between variable decls block and the body, please.
(In reply to comment #2) > Review of attachment 284696 [details] [review]: > > ::: src/ephy-location-controller.c > @@ +693,3 @@ > + * > + * The #EphyLocationController sets the address of this title box. > + */ > > This looks bad indented. It looks like the surrounding code? > ::: src/ephy-title-box.c > @@ +685,3 @@ > + char *uri; > + > + g_return_if_fail (EPHY_IS_TITLE_BOX (title_box)); > > do we allow a NULL address? No, it's guaranteed to be non-null. Fixed the other two.
Created attachment 284913 [details] [review] Let the location controller control the title box address
(In reply to comment #3) > > do we allow a NULL address? > > No, it's guaranteed to be non-null. Er, whoops, I'll add the assertion.
Created attachment 284914 [details] [review] Let the location controller control the title box address
Created attachment 284915 [details] [review] Let the location controller control the title box address Third time's the charm? The address can be null. The address property of the web view will never be null, but if it matches one of the do_not_show_address addresses in ephy-embed-utils.c then the location controller will receive a null address.
(In reply to comment #3) > (In reply to comment #2) > > Review of attachment 284696 [details] [review] [details]: > > > > ::: src/ephy-location-controller.c > > @@ +693,3 @@ > > + * > > + * The #EphyLocationController sets the address of this title box. > > + */ > > > > This looks bad indented. > > It looks like the surrounding code? I mean the *, they are usually lined up
Comment on attachment 284915 [details] [review] Let the location controller control the title box address Ok
(In reply to comment #8) > I mean the *, they are usually lined up Usually, but in that file they don't. :/
Attachment 284915 [details] pushed as db37eb4 - Let the location controller control the title box address
We need to not update the address in the title box if the user has typed an address manually into the location entry.
Created attachment 285615 [details] [review] Fix partially-typed address appearing in subtitle Now that the EphyLocationController manages the address of the EphyTitleBox in addition to the EphyLocationEntry, it's no longer safe to update its address when the address of the location entry changes, since we want the title box to display the address of the current page, not whatever the user has typed into the location entry. This is easy to fix by simply not subscribing to address updates from the location entry, which are no longer needed. (I think I've finally got this right. This patch leaves location entry locking in place. It will discard what the user has typed in the location entry only if the title box switches from location entry mode to title mode.)
Comment on attachment 285615 [details] [review] Fix partially-typed address appearing in subtitle This breaks autocompletion.
Created attachment 285692 [details] [review] Fix partially-typed address appearing in subtitle Now that the EphyLocationController manages the address of the EphyTitleBox in addition to the EphyLocationEntry, it's no longer safe to update its address when the address of the location entry changes, since we want the title box to display the address of the current page, not whatever the user has typed into the location entry. This is easy to fix by simply not updating the location controller's address with the location entry's address.
Comment on attachment 285692 [details] [review] Fix partially-typed address appearing in subtitle Ok, sorry for the delay.
Attachment 285692 [details] pushed as d6b767b - Fix partially-typed address appearing in subtitle