65608 - need nsIHistoryObserver interface and underlying implementation

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Judson Valeski]

http://www.mozilla.org/projects/embedding/apiReviewNotes.html#nsIHistoryObserver 
points to what we're after to provide more functional history info for 
embeddors.

Here's the latest info:

[1/4/01]

Too complicated to have embedding app impl the current session history 
interfaces. 
See nsIWebNavigation for other notes. 
We have to maintain loaded state internally. It's not enoughthat the embedding 
app maintain a list of URLs which they LoadURI() on when the user jumps around 
in the history list. 
It's not clear whether or not we should be index (number) based or cookie 
(history entry interface) based. 
Maybe the embedding app tells us *when* to trim, thus makingthe index option 
more attractive.

Comment 1

[Judson Valeski]

nominating 0.8

Comment 2

[Radha on family leave (not reading bugmail)]

Attachment: Initial patch

Comment 3

[Radha on family leave (not reading bugmail)]

I have attached a initial patch for the implementation. I'm seeking input (from
appripriate people) for certain parts of the patch. This patch has not been
built on linux yet.

Comment 4

[Radha on family leave (not reading bugmail)]

Attachment: updated patch that includes resource files and jud's checkins

Comment 5

[Judson Valeski]

I don't see the nsWebBRowser.cpp changes in there to enable session history 
observer registration from nsIWebBrowser.

Comment 6

[Radha on family leave (not reading bugmail)]

Attachment: more updated

Comment 7

[Radha on family leave (not reading bugmail)]

oops! forgot to attach nsWebBrowser.cpp diffs in the previous patch id 24170. 
The latest one 24242 has it. Thanks jud for pointing it out.

Comment 8

[Judson Valeski]

nice! r=valeski

Comment 9

[Peter "jag" Annema]

In nsISHistory.idl:

    * Add a new Entry to the History List
-       * @param aEntry - The entry to add
-       * @param aPersist - If true this specifies that the entry should persist
-       * in the list.  If false, this means that when new entries are added
-       * this element will not appear in the session history list.
+        * @param aEntry - The entry to add
+        * @param aPersist - If true this specifies that the entry should
persist
+        * in the list.  If false, this means that when new entries are added
+        * this element will not appear in the session history list.
    */

radha, would you mind fixing this so the *s lines up (since you're touching
those lines anyway)?

Will there only ever be one history listener or should we support multiple
listeners? The comments could be more clear on this, as could the method names
(setSHistoryListener or something).

Comment 10

[Radha on family leave (not reading bugmail)]

sr=rpotts (thro' email) and this got checked in yesterday. 

jag: for your question, each browser window or embedding window will have its 
own instance of SH. SH is not a service. so, it is enough if each instance have 
one listener. 

Comment 11

[Radha on family leave (not reading bugmail)]

*** Bug 28615 has been marked as a duplicate of this bug. ***

Comment 12

[Michael Dunn]

Moving API bug ownership to David Epstein.

Comment 13

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 14

[David Epstein]

Interface is now called nsISHistoryListener. Idl file found in lxr.mozilla.org 
Used in winEmbed for history notifications (in AddWebBrowserListener()).