438256 - Address bar is not updated after cancelling "navigate away from this page"

Status: RESOLVED WONTFIX

(Camino Graveyard :: Location Bar & Autocomplete, defect)

Description

[[e-mail]]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.8.1.12) Gecko/20080206 Camino/1.5.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.8.1.12) Gecko/20080206 Camino/1.5.5

When a Google Docs document has not been saved recently, and command-B is pressed mistakenly for "Bold", a dialog appears warning against navigating away from the page. The new URL is visible in the address bar: "about:bookmarks". When the action is cancelled, Camino's address bar is not reverted to that of the current page, but is left at "about:bookmarks".

Reproducible: Always

Steps to Reproduce:
1. Created new Google Docs document (any type)
2. Press Command-B
3. Cancel the action
See address bar has not updated.
Actual Results:  
The address bar is left pointing to "about:bookmarks", instead of reverting to that of the current page.

Expected Results:  
The address bar should have reverted to that of the current page.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Chris, is this something your recent location bar work will fix? If so, please dupe appropriately.

(Note on the trunk you'll have to use something like Cmd-Y to repro, since the page eats Cmd-B, and you'll have to actually type something in the new document before trying.)  

[e-mail], 1.5.5 is quite old and has known security issues; you should upgrade to 1.6.1.

Comment 2

[Chris Lawson (gone)]

I'm not sure, to tell you the truth, but I'll definitely put this on my list of stuff to investigate. I think I know where this is happening in the code, but I'm not quite sure what the right fix is just yet.

Comment 3

[Chris Lawson (gone)]

Oh, and I meant to mention -- if you select the location bar either by clicking in it or by using Cmd-L, you can hit Escape to get the proper URL back.

Comment 4

[Chris Lawson (gone)]

Attachment: fix v1

The more I think about this, the less I like bryner's "blast it into the urlbar immediately" approach. I took a look at the callers for loadURI:referrer:flags:focusContent:allowPopups: and I don't think removing this behaviour is going to break anything.

Here's the diff that instituted that code and comment:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/camino/src/browser&command=DIFF_FRAMESET&file=BrowserWrapper.mm&rev2=1.33&rev1=1.32

The other location bar out-of-sync bugs generally espouse a philosophy that the location bar's contents (including the favicon, RSS icon, and especially the security lock and yellow background colour) are sacred and should not be messed with unless

a) the user is actively typing in the location bar (only changes text; does not change any icons or BG colour), or
b) the content area changes (in which case everything gets re-synched to the new site being viewed)

In keeping with that philosophy, this seems like the Right Thing To Do™.

Comment 5

[Sean Murphy]

(In reply to comment #4)

> The other location bar out-of-sync bugs generally espouse a philosophy that the
> location bar's contents (including the favicon, RSS icon, and especially the
> security lock and yellow background colour) are sacred and should not be messed
> with unless
> 
> a) the user is actively typing in the location bar (only changes text; does not
> change any icons or BG colour), or
> b) the content area changes (in which case everything gets re-synched to the
> new site being viewed)
> 
> In keeping with that philosophy, this seems like the Right Thing To Do™.

This patch basically means that, when loading a page, the address bar will only be updated during the onLocationChange nsIWebProgressListener method.  Technically, that behavior is not incorrect, so the actual code change checks out fine, but we will need to debate whether this course of address bar updating is one we want to take.

Documentation about when onLocationChange is called:
"This is not when a load is requested, but rather once it is verified that the load is going to occur in the given window. For instance, a load that starts in a window might send progress and status messages for the new site, but it will not send the onLocationChange until we are sure that we are loading this new page here."

One particular area which I think can introduce confusion is when a page is visited (by a means other than typing into the address bar) and the server does not appear to respond.  In this case, it will take considerable time before the request times out, meaning the user will likely stop the load before an error page is displayed.  While the page wasn't actually loaded, the location would not be visible anywhere in the interface at this point, and the user thus left unable to examine or modify it.

Safari updates its address bar immediately in the case I mentioned above, but also does not display an incorrect URL when following the Google Doc STR mentioned on this bug.

My opinion is that we might want to seek another way to fix this bug, one that does not necessarily change existing behavior of our application just to fix an issue.

Comment 6

[Chris Lawson (gone)]

Comment on attachment 324556 [details] [diff] [review]
fix v1

Clearing review request until we decide what approach to take. Stuart mentioned something about a better way to do this in-channel recently but I forget what bug was involved.

Comment 7

[Gordon P. Hemsley [:GPHemsley]]

This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]