322106 - Subframe and redirected URLs stay "hidden" permanently in history

Status: RESOLVED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Andrew Schultz]

If a URL is first visited as part of a redirection or as a subframe, the URI gets marked as hidden in history.  If the URI is later visited as a top-level page without a redirect, it stays hidden (unless the user happens to type it explicitly in the URL bar).  It seems that history should unhide any hidden URI if it is top-level and without redirect.

Comment 1

[Andrew Schultz]

Attachment: patch

this is mostly just moving stuff around.

Comment 2

[Jesse Ruderman]

WFM Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060102 Firefox/1.6a1.  I might not be testing the same way you tested, though.

Comment 3

[Andrew Schultz]

I see the bug with a Dec 17 firefox CVS trunk build

My test was
1. set up a server-side redirect for foo.html => bar.html
2. visit foo.html (not by typing the URI into the location bar)
==> foo.html should not show up in history
3. disable server-side redirection
4. visit foo.html (not by typing the URI)
==> foo.html should show up in history but does not

That situation is not uncommon with sites the do cookie redirection madness.

If you type in the foo.html URI, it will always show up.  Based on the code, the same thing should happen with frames but I haven't tested it.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 207398 [details] [diff] [review]
patch

r=biesi, sorry for taking so long :(

Comment 5

[[e-mail]]

Comment on attachment 207398 [details] [diff] [review]
patch

>+    PRBool wasTyped = HasCell(mEnv, row, kToken_TypedColumn);
>+    if ((!isJavascript && !aRedirect && aTopLevel) || wasTyped) {
>+      row->CutColumn(mEnv, kToken_HiddenColumn);
>+
>+      if (wasTyped) {
I don't like this nesting. I'd prefer if the if (wasTyped) was at the same level as PRBool wasTyped, and possibly before the other if too.

>+        nsCAutoString URISpec;
>+        rv = GetRowValue(row, kToken_URLColumn, URISpec);
>+        NS_ENSURE_SUCCESS(rv, rv);
We already have the URISpec; your compiler should at least have spit out a warning at this point.

>       // if this is a JS url, or a redirected URI or in a frame, hide it in
>       // global history so that it doesn't show up in the autocomplete
>       // dropdown. AddExistingPageToDatabase has logic to override this
>       // behavior for URIs which were typed. See bug 197127 and bug 161531
>       // for details.
This comment is no longer accurate. Please update it to reflect the new reality.

Comment 6

[Andrew Schultz]

Attachment: patch v2

updated to address review comments

Comment 7

[Andrew Schultz]

fixed