Closed Bug 322106 Opened 18 years ago Closed 18 years ago

Subframe and redirected URLs stay "hidden" permanently in history


(Core Graveyard :: History: Global, defect)

Not set


(Not tracked)



(Reporter: ajschult784, Assigned: ajschult784)


(Keywords: fixed-seamonkey1.1a, Whiteboard: [sg:low])


(1 file, 1 obsolete file)

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.
Assignee: nobody → ajschult
Attached patch patch (obsolete) — Splinter Review
this is mostly just moving stuff around.
Attachment #207398 - Flags: review?(cbiesinger)
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.
Whiteboard: [sg:low]
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 on attachment 207398 [details] [diff] [review]

r=biesi, sorry for taking so long :(
Attachment #207398 - Flags: review?(cbiesinger) → review+
Attachment #207398 - Flags: superreview?(neil)
Comment on attachment 207398 [details] [diff] [review]

>+    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.
Attachment #207398 - Flags: superreview?(neil) → superreview-
Attached patch patch v2Splinter Review
updated to address review comments
Attachment #207398 - Attachment is obsolete: true
Attachment #210741 - Flags: superreview?(neil)
Attachment #210741 - Flags: superreview?(neil) → superreview+
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #210741 - Flags: approval-branch-1.8.1?(neil)
Attachment #210741 - Flags: approval-branch-1.8.1?(neil) → approval-branch-1.8.1+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.