127506 - Typed urls don't appear in global history

Status: VERIFIED FIXED

(Core Graveyard :: History: Global, defect)

Description

[Blake Ross]

Build ID: 2002022203

Steps to Reproduce:

1. Type http://www.bingo.com/ in your urlbar and press enter.
2. Wait until the page finishes loading.
3. Open the History window and note that the page doesn't appear in your global
history.

This is a problem for all non-redirect pages. The fix is simple.

Comment 1

[Blake Ross]

Attachment: patch

Comment 2

[Blake Ross]

cc'ing alec for sr.

The movement of the NotifyFindAssertions call into the new page case is
unrelated, but I didn't understand  why we did it in the existing page case. 
All we do if the page exists is update the visit count and date, and we already
NotifyChange those.  If there's a reason why we did it in both, please let me
know and I'll move it back.

Speaking of which, the issue that we NotifyFindAssertions of invalid typed urls
is still up in the air. If you have any suggestions about fixing that, I can
incorporate them into this patch.  I guess we will need some type of
AddPageInternal with an arg specifying whether or not to notify.  If the page
was typed, we don't want to NotifyFindAssertions.  We only want to do it in the
existing page && typed case (meaning the url is valid).

Comment 3

[Alec Flett]

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

ah! see, I told you we needed a AddPageInternal :)

This patch looks good though - I'm pretty sure you're right about the extra
NotifyFindAssertions() - that was probably just an oversight.

if you feel like doing the extra work for AddPageInternal, I'll be pretty quick
about sr='ing it... what you'll basically do is save us the work of firing the
assertions.. but I think that only affects performance if there are observers,
which only happen if the history window is open. 

sr=alecf on what is here

Comment 4

[Peter Trudelle]

nsbeta1+ per Nav triage team

Comment 5

[Joe Hewitt (gone)]

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

r=hewitt

Comment 6

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.9

Comment 7

[Blake Ross]

I'll do the notification work as part of some other patch, since this has
already been approved. Marking fixed.

Comment 8

[Claudius Gayle]

VERIFIED Fixed with 20020308 builds