90224 - nsISHistoryListener::OnHistoryReload() doesn't get callback

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P3)

Description

[David Epstein]

1. Apply the patch file for Tests.cpp. Contains isolated test for Reload().
2. Launch testEmbed. /mozilla/embedding/qa/testembed
3. Enter a url.
 can reverse steps 4 & 5. same result.
4. Turn on history listener. Tests > Add History Listener.
5. Reload. Tools > Test Your Method (in Tests.cpp, OnToolsTestYourMethod(),
Reload() is called with no flags, will try out with other cases).
6. Check logfile in C:/temp folder, called "TestOutput.txt". Also notice no
alert popup for reload notification.
Result: Callback for reload isn't appearing.
Expected: Currently, only have printf statements in the nsISHistoryListener
implementations, but it still should appear for OnHistoryReload().
 note: hist lstner impls are in BrowserImplHistoryLstnr.cpp. 

Here's what appears in the TestOutput logfile:

TestEmbed started up.
nsIWebBrowserChrome::SetWebBrowser().
AddWebBrowserListener(). Add History Lstnr test passed.
Reload():  w/ LOAD_FLAGS_NONE test passed.
TestEmbed shut down.

Comment 1

[David Epstein]

Attachment: Tests.cpp file incl test case for Reload() in OnToolsTestYourMethod()

Comment 2

[David Epstein]

changed qa contact to depstein

Comment 3

[Radha on family leave (not reading bugmail)]

Listener is hooked up only in nsISHistory, but not to docshell. ie., if you QI
nsISHistory to nsIWebNavigation and did a reload() off of that, you will be
notified in your listener. Reload() called from docshell and JS will not trigger
the listener. I don't believe I can take care of this for 0.9.4 as I have more
important bugs. Moving to next milestone.

Comment 4

[Radha on family leave (not reading bugmail)]

Attachment: patch 1.0

Comment 5

[Chris McAfee]

r=mcafee, make sure this works & stuff :-)

Comment 6

[rpotts (gone)]

hey radha,

can nsDocShell::Reload() just delegate to nsSHistory::Reload() via
nsIWebNavigation just like the rest of the history methods in the docShell?

it scares me that we have two *different* implementations of Reload() ;-)  it
seems like these two implementations should be consolidated...

-- rick

Comment 7

[rpotts (gone)]

Comment on attachment 66973 [details] [diff] [review]
patch 1.0

sr=[e-mail]
(after talking with radha)

Comment 8

[Radha on family leave (not reading bugmail)]

Fix checked in. 

Comment 9

[David Epstein]

Mozilla 0.9.8 Gecko/20020209. verified.