Move nsIDocShellTreeItem::childOffset to nsDocShell and remove all uses except internally by nsDocShell (it should be removed eventually). b=376562 r=Olli.Pettay sr=bzbarsky
authormats.palmgren@bredband.net
Thu, 17 May 2007 20:49:14 -0700
changeset 1585 633138510b5ae5e25a3bfe66d2db572dc361d448
parent 1584 8591ea1e8b29517206153f6e882f037a7989f914
child 1586 f778d3406405beace82ab4c4d84b55a22d63af2b
push idunknown
push userunknown
push dateunknown
reviewersOlli.Pettay, bzbarsky
bugs376562
milestone1.9a5pre
Move nsIDocShellTreeItem::childOffset to nsDocShell and remove all uses except internally by nsDocShell (it should be removed eventually). b=376562 r=Olli.Pettay sr=bzbarsky
content/events/src/nsEventStateManager.cpp
content/events/src/nsEventStateManager.h
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
docshell/base/nsIDocShell.idl
docshell/base/nsIDocShellTreeItem.idl
embedding/browser/webBrowser/nsWebBrowser.cpp
--- a/content/events/src/nsEventStateManager.cpp
+++ b/content/events/src/nsEventStateManager.cpp
@@ -1147,17 +1147,17 @@ nsEventStateManager::PreHandleEvent(nsPr
       if (keyEvent->isAlt)
         modifierMask |= NS_MODIFIER_ALT;
       if (keyEvent->isMeta)
         modifierMask |= NS_MODIFIER_META;
 
       // Prevent keyboard scrolling while an accesskey modifier is in use.
       if (modifierMask && (modifierMask == sChromeAccessModifier ||
                            modifierMask == sContentAccessModifier))
-        HandleAccessKey(aPresContext, keyEvent, aStatus, -1,
+        HandleAccessKey(aPresContext, keyEvent, aStatus, nsnull,
                         eAccessKeyProcessingNormal, modifierMask);
     }
   case NS_KEY_DOWN:
   case NS_KEY_UP:
     {
       if (mCurrentFocus) {
         mCurrentTargetContent = mCurrentFocus;
       }
@@ -1244,25 +1244,21 @@ GetAccessModifierMask(nsISupports* aDocS
   case nsIDocShellTreeItem::typeContent:
     return sContentAccessModifier;
 
   default:
     return -1; // invalid modifier
   }
 }
 
-// Note: for the in parameter aChildOffset,
-// -1 stands for not bubbling from the child docShell
-// 0 -- childCount - 1 stands for the child docShell's offset
-// which bubbles up the access key handling
 void
 nsEventStateManager::HandleAccessKey(nsPresContext* aPresContext,
                                      nsKeyEvent *aEvent,
                                      nsEventStatus* aStatus,
-                                     PRInt32 aChildOffset,
+                                     nsIDocShellTreeItem* aBubbledFrom,
                                      ProcessingAccessKeyState aAccessKeyState,
                                      PRInt32 aModifierMask)
 {
   nsCOMPtr<nsISupports> pcContainer = aPresContext->GetContainer();
 
   // Alt or other accesskey modifier is down, we may need to do an accesskey
   if (mAccessKeys && aModifierMask == GetAccessModifierMask(pcContainer)) {
     // Someone registered an accesskey.  Find and activate it.
@@ -1288,78 +1284,74 @@ nsEventStateManager::HandleAccessKey(nsP
       NS_WARNING("no docShellTreeNode for presContext");
       return;
     }
 
     PRInt32 childCount;
     docShell->GetChildCount(&childCount);
     for (PRInt32 counter = 0; counter < childCount; counter++) {
       // Not processing the child which bubbles up the handling
-      if (aAccessKeyState == eAccessKeyProcessingUp && counter == aChildOffset)
+      nsCOMPtr<nsIDocShellTreeItem> subShellItem;
+      docShell->GetChildAt(counter, getter_AddRefs(subShellItem));
+      if (aAccessKeyState == eAccessKeyProcessingUp &&
+          subShellItem == aBubbledFrom)
         continue;
 
-      nsCOMPtr<nsIDocShellTreeItem> subShellItem;
-      nsCOMPtr<nsIPresShell> subPS;
-      nsCOMPtr<nsPresContext> subPC;
-
-      docShell->GetChildAt(counter, getter_AddRefs(subShellItem));
       nsCOMPtr<nsIDocShell> subDS = do_QueryInterface(subShellItem);
       if (subDS && IsShellVisible(subDS)) {
+        nsCOMPtr<nsIPresShell> subPS;
         subDS->GetPresShell(getter_AddRefs(subPS));
 
         // Docshells need not have a presshell (eg. display:none
         // iframes, docshells in transition between documents, etc).
         if (!subPS) {
           // Oh, well.  Just move on to the next child
           continue;
         }
 
         nsPresContext *subPC = subPS->GetPresContext();
 
         nsEventStateManager* esm =
           NS_STATIC_CAST(nsEventStateManager *, subPC->EventStateManager());
 
         if (esm)
-          esm->HandleAccessKey(subPC, aEvent, aStatus, -1,
+          esm->HandleAccessKey(subPC, aEvent, aStatus, nsnull,
                                eAccessKeyProcessingDown, aModifierMask);
 
         if (nsEventStatus_eConsumeNoDefault == *aStatus)
           break;
       }
     }
   }// if end . checking all sub docshell ends here.
 
-  // bubble up the process to the parent docShell if necessary
+  // bubble up the process to the parent docshell if necessary
   if (eAccessKeyProcessingDown != aAccessKeyState && nsEventStatus_eConsumeNoDefault != *aStatus) {
     nsCOMPtr<nsIDocShellTreeItem> docShell(do_QueryInterface(pcContainer));
     if (!docShell) {
-      NS_WARNING("no docShellTreeNode for presContext");
+      NS_WARNING("no docShellTreeItem for presContext");
       return;
     }
 
     nsCOMPtr<nsIDocShellTreeItem> parentShellItem;
     docShell->GetParent(getter_AddRefs(parentShellItem));
     nsCOMPtr<nsIDocShell> parentDS = do_QueryInterface(parentShellItem);
     if (parentDS) {
-      PRInt32 myOffset;
-      docShell->GetChildOffset(&myOffset);
-
       nsCOMPtr<nsIPresShell> parentPS;
 
       parentDS->GetPresShell(getter_AddRefs(parentPS));
       NS_ASSERTION(parentPS, "Our PresShell exists but the parent's does not?");
 
       nsPresContext *parentPC = parentPS->GetPresContext();
       NS_ASSERTION(parentPC, "PresShell without PresContext");
 
       nsEventStateManager* esm =
         NS_STATIC_CAST(nsEventStateManager *, parentPC->EventStateManager());
 
       if (esm)
-        esm->HandleAccessKey(parentPC, aEvent, aStatus, myOffset,
+        esm->HandleAccessKey(parentPC, aEvent, aStatus, docShell,
                              eAccessKeyProcessingUp, aModifierMask);
     }
   }// if end. bubble up process
 }// end of HandleAccessKey
 
 
 #ifdef CLICK_HOLD_CONTEXT_MENUS
 
--- a/content/events/src/nsEventStateManager.h
+++ b/content/events/src/nsEventStateManager.h
@@ -220,25 +220,48 @@ protected:
 
   PRInt32 GetNextTabIndex(nsIContent* aParent, PRBool foward);
   nsresult SendFocusBlur(nsPresContext* aPresContext, nsIContent *aContent, PRBool aEnsureWindowHasFocus);
   void EnsureDocument(nsIPresShell* aPresShell);
   void EnsureDocument(nsPresContext* aPresContext);
   void FlushPendingEvents(nsPresContext* aPresContext);
   nsIFocusController* GetFocusControllerForDocument(nsIDocument* aDocument);
 
+  /**
+   * The phases of HandleAccessKey processing. See below.
+   */
   typedef enum {
     eAccessKeyProcessingNormal = 0,
     eAccessKeyProcessingUp,
     eAccessKeyProcessingDown
   } ProcessingAccessKeyState;
+
+  /**
+   * Access key handling.  If there is registered content for the accesskey
+   * given by the key event and modifier mask then call
+   * content.PerformAccesskey(), otherwise call HandleAccessKey() recursively,
+   * on descendant docshells first, then on the ancestor (with |aBubbledFrom|
+   * set to the docshell associated with |this|), until something matches.
+   *
+   * @param aPresContext the presentation context
+   * @param aEvent the key event
+   * @param aStatus the event status
+   * @param aBubbledFrom is used by an ancestor to avoid calling HandleAccessKey()
+   *        on the child the call originally came from, i.e. this is the child
+   *        that recursively called us in it's Up phase. The initial caller
+   *        passes |nsnull| here. This is to avoid an infinite loop.
+   * @param aAccessKeyState Normal, Down or Up processing phase (see enums
+   *        above). The initial event reciever uses 'normal', then 'down' when
+   *        processing children and Up when recursively calling its ancestor.
+   * @param aModifierMask modifier mask for the key event
+   */
   void HandleAccessKey(nsPresContext* aPresContext,
                        nsKeyEvent* aEvent,
                        nsEventStatus* aStatus,
-                       PRInt32 aChildOffset,
+                       nsIDocShellTreeItem* aBubbledFrom,
                        ProcessingAccessKeyState aAccessKeyState,
                        PRInt32 aModifierMask);
 
   //---------------------------------------------
   // DocShell Focus Traversal Methods
   //---------------------------------------------
 
   nsresult ShiftFocusInternal(PRBool aForward, nsIContent* aStart = nsnull);
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -20,16 +20,17 @@
  * Portions created by the Initial Developer are Copyright (C) 1999
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *   Travis Bogard <travis@netscape.com>
  *   Pierre Phaneuf <pp@ludusdesign.com>
  *   Peter Annema <disttsc@bart.nl>
  *   Dan Rosen <dr@netscape.com>
+ *   Mats Palmgren <mats.palmgren@bredband.net>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
  * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -2186,30 +2187,22 @@ nsDocShell::SetTreeOwner(nsIDocShellTree
         if (childType == mItemType)
             child->SetTreeOwner(aTreeOwner);
     }
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsDocShell::SetChildOffset(PRInt32 aChildOffset)
+nsDocShell::SetChildOffset(PRUint32 aChildOffset)
 {
     mChildOffset = aChildOffset;
     return NS_OK;
 }
 
-NS_IMETHODIMP
-nsDocShell::GetChildOffset(PRInt32 * aChildOffset)
-{
-    NS_ENSURE_ARG_POINTER(aChildOffset);
-    *aChildOffset = mChildOffset;
-    return NS_OK;
-}
-
 //*****************************************************************************
 // nsDocShell::nsIDocShellTreeNode
 //*****************************************************************************   
 
 NS_IMETHODIMP
 nsDocShell::GetChildCount(PRInt32 * aChildCount)
 {
     NS_ENSURE_ARG_POINTER(aChildCount);
@@ -2243,21 +2236,27 @@ nsDocShell::AddChild(nsIDocShellTreeItem
     }
 
     // Make sure to clear the treeowner in case this child is a different type
     // from us.
     aChild->SetTreeOwner(nsnull);
     
     nsresult res = AddChildLoader(childAsDocLoader);
     NS_ENSURE_SUCCESS(res, res);
+    NS_ASSERTION(mChildList.Count() > 0,
+                 "child list must not be empty after a successful add");
 
     // Set the child's index in the parent's children list 
     // XXX What if the parent had different types of children?
     // XXX in that case docshell hierarchy and SH hierarchy won't match.
-    aChild->SetChildOffset(mChildList.Count() - 1);
+    {
+        nsCOMPtr<nsIDocShell> childDocShell = do_QueryInterface(aChild);
+        if (childDocShell)
+            childDocShell->SetChildOffset(mChildList.Count() - 1);
+    }
 
     /* Set the child's global history if the parent has one */
     if (mGlobalHistory) {
         nsCOMPtr<nsIDocShellHistory>
             dsHistoryChild(do_QueryInterface(aChild));
         if (dsHistoryChild)
             dsHistoryChild->SetUseGlobalHistory(PR_TRUE);
     }
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -530,17 +530,26 @@ protected:
     // Indicates to CreateContentViewer() that it is safe to cache the old
     // presentation of the page, and to SetupNewViewer() that the old viewer
     // should be passed a SHEntry to save itself into.
     PRPackedBool               mSavingOldViewer;
 
     PRUint32                   mAppType;
 
     // Offset in the parent's child list.
-    PRInt32                    mChildOffset;
+    // XXXmats the line above is bogus, it's the offset in the parent's
+    // child list at the time this docshell was added to it,
+    // see nsDocShell::AddChild().  It isn't updated after that so if children
+    // with lower indices are removed this offset is no longer valid to be used
+    // as an index into the parent's child list (see bug 162283).  It MUST not
+    // be used for that purpose.  It's used as an index to get/add history
+    // entries into nsIDocShellHistory, although I very much doubt that it
+    // can be correct for that purpose as well...
+    // Try not to use it, we should get rid of it.
+    PRUint32                   mChildOffset;
 
     PRUint32                   mBusyFlags;
 
     PRInt32                    mMarginWidth;
     PRInt32                    mMarginHeight;
     PRInt32                    mItemType;
 
     PRUint32                   mLoadType;
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -63,17 +63,17 @@ interface nsIWebNavigation;
 interface nsISimpleEnumerator;
 interface nsIInputStream;
 interface nsIRequest;
 interface nsISHEntry;
 interface nsILayoutHistoryState;
 interface nsISecureBrowserUI;
 interface nsIDOMStorage;
 
-[scriptable, uuid(fbe2a673-4b8b-46a2-b225-398c52cc05cb)]
+[scriptable, uuid(db67b973-ba1a-49fa-b5b4-7670d203fa0e)]
 interface nsIDocShell : nsISupports
 {
   /**
    * Loads a given URI.  This will give priority to loading the requested URI
    * in the object implementing	this interface.  If it can't be loaded here
    * however, the URL dispatcher will go through its normal process of content
    * loading.
    *
@@ -428,10 +428,15 @@ interface nsIDocShell : nsISupports
   void addSessionStorage(in ACString aDomain, in nsIDOMStorage storage);
 
   /**
    * Gets the channel for the currently loaded document, if any. 
    * For a new document load, this will be the channel of the previous document
    * until after OnLocationChange fires.
    */
   readonly attribute nsIChannel currentDocumentChannel;
+
+  /**
+   * Set the offset of this child in its container.
+   */
+  [noscript] void setChildOffset(in unsigned long offset);
 };
 
--- a/docshell/base/nsIDocShellTreeItem.idl
+++ b/docshell/base/nsIDocShellTreeItem.idl
@@ -43,17 +43,17 @@ interface nsIDocShellTreeOwner;
 
 
 /**
  * The nsIDocShellTreeItem supplies the methods that are required of any item
  * that wishes to be able to live within the docshell tree either as a middle
  * node or a leaf. 
  */
 
-[scriptable, uuid(377d6996-c703-497d-9330-536562fcfff5)]
+[scriptable, uuid(09b54ec1-d98a-49a9-bc95-3219e8b55089)]
 interface nsIDocShellTreeItem : nsIDocShellTreeNode
 {
 	/*
 	name of the DocShellTreeItem
 	*/
 	attribute wstring name;
 
         /**
@@ -152,13 +152,10 @@ interface nsIDocShellTreeItem : nsIDocSh
 	Implementers of this interface are guaranteed that when treeOwner is
 	set that the poitner is valid without having to addref.
 	
 	Further note however when others try to get the interface it should be 
 	addref'd before handing it to them. 
 	*/
 	readonly attribute nsIDocShellTreeOwner treeOwner;
 	[noscript] void setTreeOwner(in nsIDocShellTreeOwner treeOwner);
-
-	/* The offset of yourself in your parent's child list */
-	attribute long childOffset;
 };
 
--- a/embedding/browser/webBrowser/nsWebBrowser.cpp
+++ b/embedding/browser/webBrowser/nsWebBrowser.cpp
@@ -608,28 +608,16 @@ NS_IMETHODIMP nsWebBrowser::GetTreeOwner
 }
 
 NS_IMETHODIMP nsWebBrowser::SetTreeOwner(nsIDocShellTreeOwner* aTreeOwner)
 {
    NS_ENSURE_SUCCESS(EnsureDocShellTreeOwner(), NS_ERROR_FAILURE);
    return mDocShellTreeOwner->SetTreeOwner(aTreeOwner);
 }
 
-NS_IMETHODIMP nsWebBrowser::SetChildOffset(PRInt32 aChildOffset)
-{
-  // Not implemented
-  return NS_OK;
-}
-
-NS_IMETHODIMP nsWebBrowser::GetChildOffset(PRInt32 *aChildOffset)
-{
-  // Not implemented
-  return NS_OK;
-}
-
 //*****************************************************************************
 // nsWebBrowser::nsIDocShellTreeItem
 //*****************************************************************************
 
 NS_IMETHODIMP nsWebBrowser::GetChildCount(PRInt32 * aChildCount)
 {
     NS_ENSURE_ARG_POINTER(aChildCount);
     *aChildCount = 0;