Re-factor nsISHistory and nsISHEntry as per discussions posted at http://www.mozilla.org/projects/embedding/apiReviewNotes.html and n.p.m.embedding

I have a initial set of diffs. The diffs don't build on linux. The diffs have the following set of changes: 1) A new nsIHistoryentry interface 2) nsISHistory refactored in to nsISHistory and nsISHistoryInternal and the methods divided among the two. 3) Changes to nsSHistory.cpp, nsSHEntry.cpp, nsWebShell.cpp, nsDocShell.cpp, nsHistory.cpp due to the change in intercaces. 4) Removed the unused child enumerator in nsISHContainer.idl and nsSHEntry.cpp 5) Added a enumerator to go thro' the complete historylist to nsSHistory.cpp as part of the specifications 6) Removed some unsed methods in nsSHistory (GetNextEntry(), GetPrevEntry()) 7) Removed a unused function UpdateCurrentSessionHistory() in nsDocShell.cpp Back/forward/Go functionalities work. Things to be done: 1) The flag that indicates frameset navigation in nsIHistoryEntry is yet to be implemented 2) Write some test cases or modify nsSHistory::printHistory() to use the newly added enumerator. May be this can be an exercise for our QA team. 3) I noticed few tab problems. Take care of that 4) Get it to build and run in linux. 5) Other things that the reviewers may notice. Please note that these are just initial set of changes. The basic things work.

The latest patch takes care of most of the TBDs in the previous patch. 1) Builds on linux/solaris 2) Subframe navigation flag working 3) Tab problems fixed. 4) Leaving the QA of the SHEnumerator to David Epstein. Discussed the same with him. Can I get a review/super review? It is a long patch.Please bear with it and try to provide a review. Thanks,

I'm not an expert on session history so forgive my superficial comments. Here's one: The session history enumerator descends from nsIEnumerator. See this comment: http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsIEnumerator.idl#36. Shouldn't it use nsISimpleEnumerator?

Conrad, you are right, I will change it over to nsISimpleEnumerator. Most of the changes in the patch are mundane ones, ie., changing nsISHistory to nsISHistoryInternal or changing nsISHEntry to nsIHistoryEntry.

Comments from rpotts (in a email) with some suggestions. Shall take care of the suggestions: (r/sr=rpotts) -- which ever you need :-)

/cvsroot/mozilla/xpfe/components/shistory/public/MANIFEST_IDL,v - * "\ No newline at end of file" - make sure there are newlines at the end of this. nsIHistoryEntry - * "//readonly attribute boolean isSubFrame;" - The review notes indicate that we need this attribute. Why is it commented out? * Can you add the "@status FROZEN" and "@version 1.0" tokens to the interface comment (after we resolve the isSubFrame() attribute)? We're now using these to indicate interface status. "@status UNDER_REVIEW" is used when an interface is in flux, but we want to make it public. nsISHEntry.idl - There are methods which are commented out here. We should just remove them if they're not used.

fixed last night

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

verified: 1. nsIHistoryEntry.idl exists. 2. nsISHistoryInternal iface spun off from nsIHistoryEntry. 3. no child enumerator in nsISHContainer.idl or nsSHEntry.cpp (there are child counters). 4. history enumerators in nsSHistory.cpp. nsSHEnumerator implementations 5. GetNextEntry(), GetPrevEntry() removed from nsSHistory.cpp. 6. UpdateCurrentSessionHistory() removed from nsDocShell.cpp 7. flag in nsIHistoryEntry.idl for subframe nav. 8. printHistory() method in nsSHistory.cpp. Didn't see any tests with enumerator but QA will take care of this.