GNOME Bugzilla – Bug 119047
ephy_embed_get_location() gives wrong answer
Last modified: 2005-08-01 21:21:00 UTC
After the "ge_location" signal of the embed has delivered a new address for the embed, the ephy_embed_get_location() function still returns the old address.
This still occurs even with the recent EphyBrowser changes. I've investigated this some more: In mozilla-embed.cpp:mozilla_embed_location_changed_cb() we the location from gtk_moz_embed_get_location() when emitting the signal. We get in _location_changed_cb when the gtkmozembed emits "location" signal. This happens from (gtkmozembed's) EmbedProgress::OnLocationChange. It stores the new uri and emits the signal. On the other hand, ephy_embed_get_location ultimately uses EphyBrowser::GetDocumentUrl, which uses the nsIDOMDocument's nsIDocument's GetDocumentUrl. Apparently this still gives the location from before the EmbedProgress::OnLocationChange. Should we use gtk_moz_embed_get_location in mozilla-embed.cpp:impl_get_location[ at least for toplevel == TRUE case] ?
If I'm not wrong the difference between the two is that the gtk embed one is exactly what we passed to load_url, the EphyBrowse one is the actual url loaded in the browser.
So I think it happens because on location signal the new dom document has not been yet loaded. Gtkmozembed set his uri internally from the call it gets in ProgressListener. mOwner->SetURI(newURI.get()); gtk_signal_emit(GTK_OBJECT(mOwner->mOwningWidget), moz_embed_signals[LOCATION]); I think the only solution here is to change the embed signal to pass the location in the callback itself, and to use the mozembed api in that case only to get the url. If you have better ideas...
I think we should really change the api here. The document uri and the loading uri are really different things. Maybe add flags instead of the boolean ? EPHY_EMBED_TOPLEVEL_DOCUMENT EPHY_EMBED_FOCUSED_DOCUMENT EPHY_EMBED_LOADING_DOCUMENT While at it we should also rename to get_address. Chritian, what do you think ?
Yes, the proposed API makes much more sense than what we have now.
> EPHY_EMBED_TOPLEVEL_DOCUMENT > EPHY_EMBED_FOCUSED_DOCUMENT > EPHY_EMBED_LOADING_DOCUMENT From a plugin developer's point of view, this seems annoying: every time I want the location, I'll have to try EPHY_EMBED_LOADING_DOCUMENT and then EPHY_EMBED_TOPLEVEL_DOCUMENT in sequence. Either that or we could use some renaming. Here's my proposition: EPHY_EMBED_DISPLAYED_TOPLEVEL_LOCATION EPHY_EMBED_DISPLAYED_FOCUSED_LOCATION EPHY_EMBED_LOCATION Yeah, the names are longer... but IMO they carry more appropriate meaning.
WONTFIX again. It seem easier to just have an address argument in signals.
Reopening again :) We need to solve this once and for all times, to fix bug 126167 and bug 147840.
I think this is fixed, can someone confirm?
Confirmed fixed.