Bug 472090, change scale to use a listener instead of mutation events, r=smaug,sr=neil
authorNeil Deakin <neil@mozilla.com>
Wed, 14 Jan 2009 13:21:58 -0500
changeset 23681 fa4b6ada095da4b5aef07f8a5a7f4d146273efa1
parent 23680 1c0cbd183902ee35ae48bfbbb7a17b6e0ce473db
child 23682 fa2d1d0b9345a0f9cdb32655ddc827beff6a3926
push id4675
push userneil@mozilla.com
push dateWed, 14 Jan 2009 18:22:43 +0000
treeherdermozilla-central@fa4b6ada095d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, neil
bugs472090
milestone1.9.2a1pre
Bug 472090, change scale to use a listener instead of mutation events, r=smaug,sr=neil
layout/xul/base/public/Makefile.in
layout/xul/base/public/nsISliderListener.idl
layout/xul/base/src/nsIScrollbarListener.h
layout/xul/base/src/nsSliderFrame.cpp
layout/xul/base/src/nsSliderFrame.h
toolkit/content/widgets/scale.xml
--- a/layout/xul/base/public/Makefile.in
+++ b/layout/xul/base/public/Makefile.in
@@ -58,12 +58,13 @@ XPIDLSRCS=      nsIBoxObject.idl        
                 nsIScrollBoxObject.idl \
                 nsIPopupBoxObject.idl \
                 nsIMenuBoxObject.idl \
                 nsIBrowserBoxObject.idl \
                 nsIIFrameBoxObject.idl \
                 nsIEditorBoxObject.idl \
                 nsIContainerBoxObject.idl \
                 nsIListBoxObject.idl \
+                nsISliderListener.idl \
                 $(NULL)
 
 include $(topsrcdir)/config/rules.mk
 
new file mode 100644
--- /dev/null
+++ b/layout/xul/base/public/nsISliderListener.idl
@@ -0,0 +1,52 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is Neil Deakin
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *
+ * 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
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "nsISupports.idl"
+
+/**
+ * Used for <scale> to listen to slider changes to avoid mutation listeners
+ */
+[scriptable, uuid(CD380CB5-E9F6-4B7D-A19F-B1FD7B31C532)]
+interface nsISliderListener : nsISupports
+{
+  /**
+   * Called when the current, minimum or maximum value has been changed to
+   * newValue. The which parameter will either be 'curpos', 'minpos' or 'maxpos'.
+   * If userChanged is true, then the user changed ths slider, otherwise it
+   * was changed via some other means.
+   */
+  void valueChanged(in AString which, in long newValue, in boolean userChanged);
+};
deleted file mode 100644
--- a/layout/xul/base/src/nsIScrollbarListener.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is Mozilla Communicator client code.
- *
- * The Initial Developer of the Original Code is
- * Netscape Communications Corporation.
- * Portions created by the Initial Developer are Copyright (C) 1998
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- *   Original Author: David W. Hyatt (hyatt@netscape.com)
- *
- * 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
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-#ifndef nsIScrollbarListener_h___
-#define nsIScrollbarListener_h___
-
-// {A0ADBD81-2911-11d3-97FA-00400553EEF0}
-#define NS_ISCROLLBARLISTENER_IID \
-{ 0xa0adbd81, 0x2911, 0x11d3, { 0x97, 0xfa, 0x0, 0x40, 0x5, 0x53, 0xee, 0xf0 } }
-
-static NS_DEFINE_IID(kIScrollbarListenerIID,     NS_ISCROLLBARLISTENER_IID);
-
-class nsPresContext;
-
-class nsIScrollbarListener : public nsISupports {
-
-public:
-
-  NS_DECLARE_STATIC_IID_ACCESSOR(NS_ISCROLLBARLISTENER_IID)
-  
-  NS_IMETHOD PositionChanged(nsPresContext* aPresContext, PRInt32 aOldIndex, PRInt32& aNewIndex) = 0;
-
-  NS_IMETHOD PagedUpDown() = 0;
-};
-
-NS_DEFINE_STATIC_IID_ACCESSOR(nsIScrollbarListener, NS_ISCROLLBARLISTENER_IID)
-
-#endif
-
--- a/layout/xul/base/src/nsSliderFrame.cpp
+++ b/layout/xul/base/src/nsSliderFrame.cpp
@@ -54,17 +54,17 @@
 #include "nsHTMLParts.h"
 #include "nsIPresShell.h"
 #include "nsCSSRendering.h"
 #include "nsIDOMEventTarget.h"
 #include "nsIViewManager.h"
 #include "nsIDOMMouseEvent.h"
 #include "nsIDocument.h"
 #include "nsScrollbarButtonFrame.h"
-#include "nsIScrollbarListener.h"
+#include "nsISliderListener.h"
 #include "nsIScrollbarMediator.h"
 #include "nsIScrollbarFrame.h"
 #include "nsILookAndFeel.h"
 #include "nsRepeatService.h"
 #include "nsBoxLayoutState.h"
 #include "nsSprocketLayout.h"
 #include "nsIServiceManager.h"
 #include "nsGUIEvent.h"
@@ -110,18 +110,18 @@ nsIFrame*
 NS_NewSliderFrame (nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
   return new (aPresShell) nsSliderFrame(aPresShell, aContext);
 } // NS_NewSliderFrame
 
 nsSliderFrame::nsSliderFrame(nsIPresShell* aPresShell, nsStyleContext* aContext):
   nsBoxFrame(aPresShell, aContext),
   mCurPos(0),
-  mScrollbarListener(nsnull),
-  mChange(0)
+  mChange(0),
+  mUserChanged(PR_FALSE)
 {
 }
 
 // stop timer
 nsSliderFrame::~nsSliderFrame()
 {
 }
 
@@ -223,16 +223,40 @@ nsSliderFrame::GetIntegerAttribute(nsICo
 
       // convert it to an integer
       defaultValue = value.ToInteger(&error);
     }
 
     return defaultValue;
 }
 
+class nsValueChangedRunnable : public nsRunnable
+{
+public:
+  nsValueChangedRunnable(nsISliderListener* aListener,
+                         nsIAtom* aWhich,
+                         PRInt32 aValue,
+                         PRBool aUserChanged)
+  : mListener(aListener), mWhich(aWhich),
+    mValue(aValue), mUserChanged(aUserChanged)
+  {}
+
+  NS_IMETHODIMP Run()
+  {
+    nsAutoString which;
+    mWhich->ToString(atom);
+    return mListener->ValueChanged(which, mValue, mUserChanged);
+  }
+
+  nsCOMPtr<nsISliderListener> mListener;
+  nsCOMPtr<nsIAtom> mWhich;
+  PRInt32 mValue;
+  PRBool mUserChanged;
+};
+
 NS_IMETHODIMP
 nsSliderFrame::AttributeChanged(PRInt32 aNameSpaceID,
                                 nsIAtom* aAttribute,
                                 PRInt32 aModType)
 {
   nsresult rv = nsBoxFrame::AttributeChanged(aNameSpaceID, aAttribute,
                                              aModType);
   // if the current position changes
@@ -246,16 +270,28 @@ nsSliderFrame::AttributeChanged(PRInt32 
       // bounds check it.
 
       nsIBox* scrollbarBox = GetScrollbar();
       nsCOMPtr<nsIContent> scrollbar;
       scrollbar = GetContentOfBox(scrollbarBox);
       PRInt32 current = GetCurrentPosition(scrollbar);
       PRInt32 min = GetMinPosition(scrollbar);
       PRInt32 max = GetMaxPosition(scrollbar);
+
+      // inform the parent <scale> that the minimum or maximum changed
+      nsIFrame* parent = GetParent();
+      if (parent) {
+        nsCOMPtr<nsISliderListener> sliderListener = do_QueryInterface(parent->GetContent());
+        if (sliderListener) {
+          nsContentUtils::AddScriptRunner(
+            new nsValueChangedRunnable(sliderListener, aAttribute,
+                                       aAttribute == nsGkAtoms::minpos ? min : max, PR_FALSE));
+        }
+      }
+
       if (current < min || current > max)
       {
         if (current < min || max < min)
             current = min;
         else if (current > max)
             current = max;
 
         // set the new position and notify observers
@@ -605,19 +641,16 @@ nsSliderFrame::PageUpDown(nscoord change
   nsIBox* scrollbarBox = GetScrollbar();
   nsCOMPtr<nsIContent> scrollbar;
   scrollbar = GetContentOfBox(scrollbarBox);
 
   if (mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::dir,
                             nsGkAtoms::reverse, eCaseMatters))
     change = -change;
 
-  if (mScrollbarListener)
-    mScrollbarListener->PagedUpDown(); // Let the listener decide our increment.
-
   nscoord pageIncrement = GetPageIncrement(scrollbar);
   PRInt32 curpos = GetCurrentPosition(scrollbar);
   PRInt32 minpos = GetMinPosition(scrollbar);
   PRInt32 maxpos = GetMaxPosition(scrollbar);
 
   // get the new position and make sure it is in bounds
   PRInt32 newpos = curpos + change * pageIncrement;
   if (newpos < minpos || maxpos < minpos)
@@ -680,20 +713,27 @@ nsSliderFrame::CurrentPositionChanged(ns
      newThumbRect.y = clientRect.y + nscoord(float(pos * onePixel) * mRatio);
 
   // set the rect
   thumbFrame->SetRect(newThumbRect);
 
   // Redraw the scrollbar
   InvalidateWithFlags(clientRect, aImmediateRedraw ? INVALIDATE_IMMEDIATE : 0);
 
-  if (mScrollbarListener)
-    mScrollbarListener->PositionChanged(aPresContext, mCurPos, curpospx);
+  mCurPos = curpospx;
 
-  mCurPos = curpospx;
+  // inform the parent <scale> if it exists that the value changed
+  nsIFrame* parent = GetParent();
+  if (parent) {
+    nsCOMPtr<nsISliderListener> sliderListener = do_QueryInterface(parent->GetContent());
+    if (sliderListener) {
+      nsContentUtils::AddScriptRunner(
+        new nsValueChangedRunnable(sliderListener, nsGkAtoms::curpos, mCurPos, mUserChanged));
+    }
+  }
 
   return NS_OK;
 }
 
 static void UpdateAttribute(nsIContent* aScrollbar, nscoord aNewPos, PRBool aNotify, PRBool aIsSmooth) {
   nsAutoString str;
   str.AppendInt(aNewPos);
   
@@ -761,16 +801,19 @@ nsSliderFrame::SetCurrentPosition(nsICon
 
 void
 nsSliderFrame::SetCurrentPositionInternal(nsIContent* aScrollbar, PRInt32 aNewPos,
                                           PRBool aIsSmooth,
                                           PRBool aImmediateRedraw)
 {
   nsCOMPtr<nsIContent> scrollbar = aScrollbar;
   nsIBox* scrollbarBox = GetScrollbar();
+
+  mUserChanged = PR_TRUE;
+
   nsIScrollbarFrame* scrollbarFrame = do_QueryFrame(scrollbarBox);
   if (scrollbarFrame) {
     // See if we have a mediator.
     nsIScrollbarMediator* mediator = scrollbarFrame->GetScrollbarMediator();
     if (mediator) {
       nsRefPtr<nsPresContext> context = PresContext();
       nsCOMPtr<nsIContent> content = GetContent();
       mediator->PositionChanged(scrollbarFrame, GetCurrentPosition(scrollbar), aNewPos);
@@ -779,21 +822,23 @@ nsSliderFrame::SetCurrentPositionInterna
       nsIPresShell* shell = context->GetPresShell();
       if (shell) {
         nsIFrame* frame = shell->GetPrimaryFrameFor(content);
         if (frame && frame->GetType() == nsGkAtoms::sliderFrame) {
           static_cast<nsSliderFrame*>(frame)->
             CurrentPositionChanged(frame->PresContext(), aImmediateRedraw);
         }
       }
+      mUserChanged = PR_FALSE;
       return;
     }
   }
 
   UpdateAttribute(scrollbar, aNewPos, PR_TRUE, aIsSmooth);
+  mUserChanged = PR_FALSE;
 
 #ifdef DEBUG_SLIDER
   printf("Current Pos=%d\n",aNewPos);
 #endif
 
 }
 
 nsIAtom*
@@ -1086,23 +1131,16 @@ nsSliderFrame::EnsureOrient()
   PRBool isHorizontal = (scrollbarBox->GetStateBits() & NS_STATE_IS_HORIZONTAL) != 0;
   if (isHorizontal)
       mState |= NS_STATE_IS_HORIZONTAL;
   else
       mState &= ~NS_STATE_IS_HORIZONTAL;
 }
 
 
-void
-nsSliderFrame::SetScrollbarListener(nsIScrollbarListener* aListener)
-{
-  // Don't addref/release this, since it's actually a frame.
-  mScrollbarListener = aListener;
-}
-
 void nsSliderFrame::Notify(void)
 {
     PRBool stop = PR_FALSE;
 
     nsIFrame* thumbFrame = mFrames.FirstChild();
     if (!thumbFrame) {
       StopRepeat();
       return;
--- a/layout/xul/base/src/nsSliderFrame.h
+++ b/layout/xul/base/src/nsSliderFrame.h
@@ -42,17 +42,16 @@
 #include "nsBoxFrame.h"
 #include "prtypes.h"
 #include "nsIAtom.h"
 #include "nsCOMPtr.h"
 #include "nsITimer.h"
 #include "nsIDOMMouseListener.h"
 
 class nsString;
-class nsIScrollbarListener;
 class nsISupportsArray;
 class nsITimer;
 class nsSliderFrame;
 
 nsIFrame* NS_NewSliderFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
 class nsSliderMediator : public nsIDOMMouseListener
 {
@@ -181,18 +180,16 @@ public:
   static PRInt32 GetCurrentPosition(nsIContent* content);
   static PRInt32 GetMinPosition(nsIContent* content);
   static PRInt32 GetMaxPosition(nsIContent* content);
   static PRInt32 GetIncrement(nsIContent* content);
   static PRInt32 GetPageIncrement(nsIContent* content);
   static PRInt32 GetIntegerAttribute(nsIContent* content, nsIAtom* atom, PRInt32 defaultValue);
   void EnsureOrient();
 
-  void SetScrollbarListener(nsIScrollbarListener* aListener);
-
   virtual nsIView* GetMouseCapturer() const { return GetView(); }
 
   NS_IMETHOD HandlePress(nsPresContext* aPresContext,
                          nsGUIEvent *    aEvent,
                          nsEventStatus*  aEventStatus);
 
   NS_IMETHOD HandleMultiplePress(nsPresContext* aPresContext,
                          nsGUIEvent *    aEvent,
@@ -237,19 +234,22 @@ private:
  
   float mRatio;
 
   nscoord mDragStart;
   nscoord mThumbStart;
 
   PRInt32 mCurPos;
 
-  nsIScrollbarListener* mScrollbarListener;
-
   nscoord mChange;
   nsPoint mDestinationPoint;
   nsRefPtr<nsSliderMediator> mMediator;
 
+  // true if an attribute change has been caused by the user manipulating the
+  // slider. This allows notifications to tell how a slider's current position
+  // was changed.
+  PRBool mUserChanged;
+
   static PRBool gMiddlePref;
   static PRInt32 gSnapMultiplier;
 }; // class nsSliderFrame
 
 #endif
--- a/toolkit/content/widgets/scale.xml
+++ b/toolkit/content/widgets/scale.xml
@@ -34,17 +34,17 @@
 
     <content align="center" pack="center">
       <xul:slider anonid="slider" class="scale-slider" snap="true" flex="1"
                   xbl:inherits="disabled,orient,dir,curpos=value,minpos=min,maxpos=max,increment,pageincrement">
         <xul:thumb class="scale-thumb" xbl:inherits="disabled,orient"/>
       </xul:slider>
     </content>
     
-    <implementation implements="nsIAccessibleProvider">
+    <implementation implements="nsIAccessibleProvider, nsISliderListener">
       <property name="accessibleType" readonly="true">
         <getter>
           return Components.interfaces.nsIAccessibleProvider.XULScale;
         </getter>
       </property>
 
       <property name="value" onget="return this._getIntegerAttribute('curpos', 0);"
                              onset="return this._setIntegerAttribute('curpos', val);"/>
@@ -52,16 +52,17 @@
                            onset="return this._setIntegerAttribute('minpos', val);"/>
       <property name="max" onget="return this._getIntegerAttribute('maxpos', 100);"
                            onset="return this._setIntegerAttribute('maxpos', val);"/>
       <property name="increment" onget="return this._getIntegerAttribute('increment', 1);"
                                  onset="return this._setIntegerAttribute('increment', val);"/>
       <property name="pageIncrement" onget="return this._getIntegerAttribute('pageincrement', 10);"
                                      onset="return this._setIntegerAttribute('pageincrement', val);"/>
 
+      <field name="_userChanged">false</field>
       <field name="_sliderElement"/>
       <property name="_slider" readonly="true">
         <getter>
           if (!this._sliderElement)
             this._sliderElement = document.getAnonymousElementByAttribute(this, "anonid", "slider");
           return this._sliderElement;
         </getter>
       </property>
@@ -143,62 +144,84 @@
         <![CDATA[
           var newpos = this.value + this.pageIncrement;
           var endpos = this.max;
           this.value = (newpos < endpos) ? newpos : endpos;
         ]]>
         </body>
       </method>
 
+      <method name="valueChanged">
+        <parameter name="which"/>
+        <parameter name="newValue"/>
+        <parameter name="userChanged"/>
+        <body>
+        <![CDATA[
+          switch (which) {
+            case "curpos":
+              this.setAttribute("value", newValue);
+
+              // in the future, only fire the change event when userChanged
+              // or _userChanged is true
+              var changeEvent = document.createEvent("Events");
+              changeEvent.initEvent("change", true, true);
+              this.dispatchEvent(changeEvent);
+              break;
+
+            case "minpos":
+              this.setAttribute("min", newValue);
+              break;
+
+            case "maxpos":
+              this.setAttribute("max", newValue);
+              break;
+          }
+        ]]>
+        </body>
+      </method>
+
     </implementation>
 
     <handlers>
-      <handler event="DOMAttrModified">
-        if (event.originalTarget != this._slider)
-          return;
-
-        switch (event.attrName) {
-          case "curpos":
-            this.setAttribute("value", event.newValue);
-
-            var changeEvent = document.createEvent("Events");
-            changeEvent.initEvent("change", true, true);
-            this.dispatchEvent(changeEvent);
-            break;
-
-          case "minpos":
-            this.setAttribute("min", event.newValue);
-            break;
-
-          case "maxpos":
-            this.setAttribute("max", event.newValue);
-            break;
-        }
-      </handler>
-
       <handler event="keypress" keycode="VK_UP" preventdefault="true">
-        return (this.dir == "reverse") ? this.increase() : this.decrease();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.increase() : this.decrease();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_LEFT" preventdefault="true">
-        return (this.dir == "reverse") ? this.increase() : this.decrease();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.increase() : this.decrease();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_DOWN" preventdefault="true">
-        return (this.dir == "reverse") ? this.decrease() : this.increase();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.decrease() : this.increase();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_RIGHT" preventdefault="true">
-        return (this.dir == "reverse") ? this.decrease() : this.increase();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.decrease() : this.increase();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_PAGE_UP" preventdefault="true">
-        return (this.dir == "reverse") ? this.increasePage() : this.decreasePage();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.increasePage() : this.decreasePage();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_PAGE_DOWN" preventdefault="true">
-        return (this.dir == "reverse") ? this.decreasePage() : this.increasePage();
+        this._userChanged = true;
+        (this.dir == "reverse") ? this.decreasePage() : this.increasePage();
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_HOME" preventdefault="true">
+        this._userChanged = true;
         this.value = (this.dir == "reverse") ? this.max : this.min;
+        this._userChanged = false;
       </handler>
       <handler event="keypress" keycode="VK_END" preventdefault="true">
+        this._userChanged = true;
         this.value = (this.dir == "reverse") ? this.min : this.max;
+        this._userChanged = false;
       </handler>
     </handlers>
 
   </binding>
 </bindings>