Bug 1249061 - Improve property/attribute handling for marquees. r=smaug
authorMartijn Wargers <mwargers@mozilla.com>
Fri, 01 Apr 2016 14:24:48 +0200
changeset 291243 3b7387f7b46b1b284b28f6e6083d8b7d2414e2ba
parent 291242 b90dc48988d46ca3e26b097a1f7af05734626cb2
child 291244 db49ab7e9e4f1460553637b79eb91538a497a062
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1249061
milestone48.0a1
Bug 1249061 - Improve property/attribute handling for marquees. r=smaug
dom/tests/mochitest/bugs/test_bug1160342_marquee.html
layout/style/xbl-marquee/xbl-marquee.xml
--- a/dom/tests/mochitest/bugs/test_bug1160342_marquee.html
+++ b/dom/tests/mochitest/bugs/test_bug1160342_marquee.html
@@ -7,222 +7,331 @@ https://bugzilla.mozilla.org/show_bug.cg
   <title>Test for Bug 411103</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1160342">Mozilla Bug 1160342</a>
 <p id="display"></p>
 <div id="content">
-<marquee id="a">marquee</marquee>
+<marquee id="a" style="border: 1px solid black;">marquee</marquee>
 </div>
 
 <pre id="test">
 <script class="testbody" type="text/javascript">
+/* The todos are cases where IE/Edge is throwing errors, but
+   for Mozilla it was decided to not do that for now */
   var x=document.getElementById('a');
 
   SimpleTest.waitForExplicitFinish();
 
   setTimeout(function() {
     is(x.behavior, "scroll", "Wrong behavior value");
     x.setAttribute('behavior', 'alternate');
     is(x.behavior, "alternate", "Wrong behavior value");
     x.setAttribute('behavior', 'invalid');
-    todo_is(x.behavior, "scroll", "Wrong behavior value");;
+    is(x.behavior, "scroll", "Wrong behavior value");;
     x.setAttribute('behavior', 'Scroll');
     is(x.behavior, "scroll", "Wrong behavior value");
     x.setAttribute('behavior', 'Slide');
     is(x.behavior, "slide", "Wrong behavior value");
+    x.setAttribute('behavior', '');
+    is(x.behavior, "scroll", "Wrong behavior value");
+    x.setAttribute('behavior', 'slide');
     x.removeAttribute('behavior');
     is(x.behavior, "scroll", "Wrong behavior value");
+    is(x.getAttribute('behavior'), null, "Wrong behavior attribute");
 
     x.behavior = 'alternate';
     is(x.behavior, "alternate", "Wrong behavior value");
     try {
       x.behavior = 'invalid';
       todo_is(false, true, "marquee.behavior = 'invalid' should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.behavior, "alternate", "Wrong behavior value");
+    is(x.getAttribute('behavior'), "alternate", "Wrong behavior attribute");
     x.behavior = 'Slide';
     is(x.behavior, "slide", "Wrong behavior value");
+    is(x.getAttribute('behavior'), "slide", "Wrong behavior attribute");
     try {
       x.behavior = 'invalid';
       todo_is(false, true, "marquee.behavior = 'invalid' should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
+    try {
+      x.behavior = null;
+      x.behavior = undefined;
+      todo_is(false, true, "marquee.behavior = 'invalid' should throw");
+    } catch(e) {
+      ok(true, "Exception was raised");
+    }
     is(x.behavior, "slide", "Wrong behavior value");
+    is(x.getAttribute('behavior'), "slide", "Wrong behavior attribute");
+    // This doesn't work in Mozilla due to chrome XBL security issues
+    x.behavior = { toString: function _toString() { return "scroll"} }
+    is(x.behavior, x.getAttribute('behavior'), "Wrong behavior value");
+    x.behavior = 'scroll';
+    is(x.behavior, "scroll", "Wrong behavior value");
 
     is(x.loop, -1, "Wrong loop value");
     x.setAttribute('loop', '1');
     is(x.loop, 1, "Wrong loop value");
     x.setAttribute('loop', 'invalid');
-    todo_is(x.loop, -1, "Wrong loop value");
+    is(x.loop, -1, "Wrong loop value");
+    x.setAttribute('loop', '');
+    is(x.loop, -1, "Wrong loop value");
+    x.setAttribute('loop', '0');
+    is(x.loop, -1, "Wrong loop value");
     x.setAttribute('loop', '1000');
     is(x.loop, 1000, "Wrong loop value");
+    x.setAttribute('loop', '-0.123');
+    is(x.loop, 1000, "Wrong loop value");
+    x.setAttribute('loop', '-1.123');
+    is(x.loop, -1, "Wrong loop value");
     x.setAttribute('loop', '-1');
     is(x.loop, -1, "Wrong loop value");
     x.setAttribute('loop', '1000');
     is(x.loop, 1000, "Wrong loop value");
     x.removeAttribute('loop');
     is(x.loop, -1, "Wrong loop value");
+    is(x.getAttribute('loop'), null, "Wrong loop attribute");
 
     x.loop = 1;
     is(x.loop, 1, "Wrong loop value");
+    is(x.getAttribute('loop'), "1", "Wrong loop attribute");
     try {
       x.loop = -2;
       todo_is(false, true, "marquee.loop = -2 should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
-    todo_is(x.loop, 1, "Wrong loop value");
+    is(x.loop, 1, "Wrong loop value");
+    is(x.getAttribute('loop'), "1", "Wrong loop attribute");
     try {
       x.loop = 'invalid';
       todo_is(false, true, ".loop = 'invalid' should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
-    todo_is(x.loop, 1, "Wrong loop value");
+    is(x.loop, 1, "Wrong loop value");
+    is(x.getAttribute('loop'), "1", "Wrong loop attribute");
     try {
       x.loop = null;
       todo_is(false, true, "marquee.loop = null should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
-    todo_is(x.loop, 1, "Wrong loop value");
+    is(x.loop, 1, "Wrong loop value");
+    is(x.getAttribute('loop'), "1", "Wrong loop attribute");
     x.loop = -1;
     is(x.loop, -1, "Wrong loop value");
+    is(x.getAttribute('loop'), "-1", "Wrong loop attribute");
     x.loop = '100';
     is(x.loop, 100, "Wrong loop value");
+    is(x.getAttribute('loop'), "100", "Wrong loop attribute");
+    try {
+      x.loop = -0.123;
+      todo_is(false, true, "marquee.loop = null should throw");
+    } catch(e) {
+      ok(true, "Exception was raised");
+    }
+    is(x.loop, 100, "Wrong loop value");
+    is(x.getAttribute('loop'), "100", "Wrong loop attribute");
+    try {
+      x.loop = 0;
+      todo_is(false, true, "marquee.loop = null should throw");
+    } catch(e) {
+      ok(true, "Exception was raised");
+    }
+    is(x.loop, 100, "Wrong loop value");
+    is(x.getAttribute('loop'), "100", "Wrong loop attribute");
+    x.loop = -1.123;
+    is(x.loop, -1, "Wrong loop value");
+    is(x.getAttribute('loop'), "-1", "Wrong loop attribute");
+
 
     is(x.scrollAmount, 6, "Wrong scrollAmount value");
     x.setAttribute('scrollAmount', '1');
     is(x.scrollAmount, 1, "Wrong scrollAmount value");
     x.setAttribute('scrollAmount', 'invalid');
-    todo_is(x.scrollAmount, 6, "Wrong scrollAmount value");
+    is(x.scrollAmount, 6, "Wrong scrollAmount value");
     x.setAttribute('scrollAmount', '1000');
     is(x.scrollAmount, 1000, "Wrong scrollAmount value");
     x.setAttribute('scrollAmount', '-1');
     is(x.scrollAmount, 1000, "Wrong scrollAmount value");
     x.setAttribute('scrollAmount', '999');
     is(x.scrollAmount, 999, "Wrong scrollAmount value");
+    x.setAttribute('scrollAmount', '');
+    is(x.scrollAmount, 6, "Wrong scrollAmount value");
+    x.setAttribute('scrollAmount', '999');
     x.removeAttribute('scrollAmount');
     is(x.scrollAmount, 6, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), null, "Wrong scrollamount attribute");
 
     x.scrollAmount = 1;
     is(x.scrollAmount, 1, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), "1", "Wrong scrolldelay attribute");
     try {
       x.scrollAmount = -2;
       todo_is(false, true, "marquee.scrollAmount = -2 should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.scrollAmount, 1, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), "1", "Wrong scrolldelay attribute");
     x.scrollAmount = 'invalid';
-    todo_is(x.scrollAmount, 0, "Wrong scrollAmount value");
+    is(x.scrollAmount, 0, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), "0", "Wrong scrolldelay attribute");
     x.scrollAmount = 1;
     x.scrollAmount = null;
-    todo_is(x.scrollAmount, 0, "Wrong scrollAmount value");
+    is(x.scrollAmount, 0, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), "0", "Wrong scrolldelay attribute");
     x.scrollAmount = '2';
     is(x.scrollAmount, 2, "Wrong scrollAmount value");
+    is(x.getAttribute('scrollamount'), "2", "Wrong scrolldelay attribute");
 
 
     is(x.scrollDelay, 85, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', '1');
     is(x.scrollDelay, 1, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', 'invalid');
-    todo_is(x.scrollDelay, 85, "Wrong scrollDelay value");
+    is(x.scrollDelay, 85, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', '70');
     is(x.scrollDelay, 70, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', '59');
     is(x.scrollDelay, 59, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', '1000');
     is(x.scrollDelay, 1000, "Wrong scrollDelay value");
     x.setAttribute('scrollDelay', '-1');
     is(x.scrollDelay, 1000, "Wrong scrollDelay value");
+    x.setAttribute('scrollDelay', '');
+    is(x.scrollDelay, 85, "Wrong scrollDelay value");
+    x.setAttribute('scrollDelay', '1000');
     x.removeAttribute('scrollDelay');
     is(x.scrollDelay, 85, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), null, "Wrong scrolldelay attribute");
 
     x.scrollDelay = 100;
     is(x.scrollDelay, 100, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "100", "Wrong scrolldelay attribute");
     try {
       x.scrollDelay = -2;
       todo_is(false, true, "marquee.scrollDelay = -2 should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.scrollDelay, 100, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "100", "Wrong scrolldelay attribute");
     try {
       x.scrollDelay = 'invalid';
       todo_is(false, true, "marquee.scrollDelay = 'invalid' should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.scrollDelay, 100, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "100", "Wrong scrolldelay attribute");
     try {
       x.scrollDelay = null;
       todo_is(false, true, "marquee.scrollDelay = null should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.scrollDelay, 100, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "100", "Wrong scrolldelay attribute");
     try {
       x.scrollDelay = -1;
       todo_is(false, true, "marquee.scrollDelay = -1 should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
     is(x.scrollDelay, 100, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "100", "Wrong scrolldelay attribute");
     x.scrollDelay = '50';
     is(x.scrollDelay, 50, "Wrong scrollDelay value");
+    is(x.getAttribute('scrolldelay'), "50", "Wrong scrolldelay attribute");
+
 
     is(x.trueSpeed, false, "Wrong trueSpeed value");
     x.setAttribute('trueSpeed', '1');
     is(x.trueSpeed, true, "Wrong trueSpeed value");
     x.setAttribute('trueSpeed', 'false');
     is(x.trueSpeed, true, "Wrong trueSpeed value");
+    x.setAttribute('trueSpeed', '');
+    is(x.trueSpeed, true, "Wrong trueSpeed value");
     x.removeAttribute('trueSpeed');
     is(x.trueSpeed, false, "Wrong trueSpeed value");
+    is(x.getAttribute('truespeed'), null, "Wrong truespeed attribute");
 
     x.trueSpeed = 1;
     is(x.trueSpeed, true, "Wrong trueSpeed value");
+    is(x.getAttribute('truespeed'), "", "Wrong truespeed attribute");
     x.trueSpeed = -2;
     is(x.trueSpeed, true, "Wrong trueSpeed value");
+    is(x.getAttribute('truespeed'), "", "Wrong truespeed attribute");
     x.trueSpeed = null;
     is(x.trueSpeed, false, "Wrong trueSpeed value");
+    is(x.getAttribute('truespeed'), null, "Wrong truespeed attribute");
     x.trueSpeed = '100';
     is(x.trueSpeed, true, "Wrong trueSpeed value");
+    is(x.getAttribute('truespeed'), "", "Wrong truespeed attribute");
 
-    todo_is(x.direction, "left", "Wrong direction value");
+
+    is(x.direction, "left", "Wrong direction value");
     x.setAttribute('direction', 'right');
     is(x.direction, "right", "Wrong direction value");
     x.setAttribute('direction', 'invalid');
-    todo_is(x.direction, "left", "Wrong direction value");
+    is(x.direction, "left", "Wrong direction value");
     x.setAttribute('direction', 'RIGHT');
-    todo_is(x.direction, "right", "Wrong direction value");
+    is(x.direction, "right", "Wrong direction value");
+    x.setAttribute('direction', '');
+    is(x.direction, "left", "Wrong direction value");
+    x.setAttribute('direction', 'right');
     x.removeAttribute('direction');
-    todo_is(x.direction, "left", "Wrong direction value");
+    is(x.direction, "left", "Wrong direction value");
+    is(x.getAttribute('direction'), null, "Wrong direction attribute");
+    x.setAttribute('direction', 'up');
+    is(x.direction, "up", "Wrong direction value");
+    x.setAttribute('direction', 'down');
+    is(x.direction, "down", "Wrong direction value");
+    x.removeAttribute('direction');
+    is(x.direction, "left", "Wrong direction value");
+    is(x.getAttribute('direction'), null, "Wrong direction attribute");
 
     x.direction = 'right';
     is(x.direction, "right", "Wrong direction value");
+    is(x.getAttribute('direction'), "right", "Wrong direction attribute");
+    x.direction = 'up';
+    is(x.direction, "up", "Wrong direction value");
+    is(x.getAttribute('direction'), "up", "Wrong direction attribute");
+    x.direction = 'down';
+    is(x.direction, "down", "Wrong direction value");
+    is(x.getAttribute('direction'), "down", "Wrong direction attribute");
     try {
       x.direction = 1;
       todo_is(false, true, "marquee.direction = 1 should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
-    todo_is(x.direction, "right", "Wrong direction value");
+    is(x.direction, "down", "Wrong direction value");
+    is(x.getAttribute('direction'), "down", "Wrong direction attribute");
     try {
       x.direction = null;
       todo_is(false, true, "marquee.direction = null should throw");
     } catch(e) {
       ok(true, "Exception was raised");
     }
-    todo_is(x.direction, "right", "Wrong direction value");
+    is(x.direction, "down", "Wrong direction value");
+    is(x.getAttribute('direction'), "down", "Wrong direction attribute");
+    // This doesn't work in Mozilla due to chrome XBL security issues
+    x.direction = { toString: function _toString() { return "right"} }
+    is(x.direction, x.getAttribute('direction'), "Wrong direction value");
+    x.direction = 'left';
+    is(x.direction, "left", "Wrong direction value");
     SimpleTest.finish();
   }, 0);
 
 </script>
 </pre>
 </body>
 </html>
--- a/layout/style/xbl-marquee/xbl-marquee.xml
+++ b/layout/style/xbl-marquee/xbl-marquee.xml
@@ -16,99 +16,126 @@
       <stylesheet src="chrome://xbl-marquee/content/xbl-marquee.css"/>
     </resources>
     <implementation>
 
       <property name="scrollAmount" exposeToUntrustedContent="true">
         <getter>
           <![CDATA[
           this._mutationActor(this._mutationObserver.takeRecords());
-          var val = parseInt(this.getAttribute("scrollamount"));
-
-          if (val <= 0 || isNaN(val))
-            return this._scrollAmount;
-
-          return val;
+          return this._scrollAmount;
           ]]>
         </getter>
         <setter>
+          <![CDATA[
+          var val = parseInt(val);
+          if (val < 0) {
+            return;
+          }
+          if (isNaN(val)) {
+            val = 0;
+          }
           this.setAttribute("scrollamount", val);
+          ]]>
         </setter>
       </property>
 
       <property name="scrollDelay" exposeToUntrustedContent="true">
         <getter>
           <![CDATA[
           this._mutationActor(this._mutationObserver.takeRecords());
           var val = parseInt(this.getAttribute("scrolldelay"));
 
-          if (val <= 0 || isNaN(val))
+          if (val <= 0 || isNaN(val)) {
             return this._scrollDelay;
+          }
 
           return val;
           ]]>
         </getter>
         <setter>
-          this.setAttribute("scrolldelay", val);
+          var val = parseInt(val);
+          if (val > 0 ) {
+            this.setAttribute("scrolldelay", val);
+          }
         </setter>
       </property>
 
       <property name="trueSpeed" exposeToUntrustedContent="true">
         <getter>
           <![CDATA[
-          if (!this.hasAttribute("truespeed"))
+          if (!this.hasAttribute("truespeed")) {
             return false;
+          }
 
           return true;
           ]]>
         </getter>
         <setter>
           <![CDATA[
-          if (val)
-            this.setAttribute("truespeed", "truespeed");
-          else
+          if (val) {
+            this.setAttribute("truespeed", "");
+          } else {
             this.removeAttribute('truespeed');
+          }
           ]]>
         </setter>
       </property>
 
       <property name="direction" exposeToUntrustedContent="true">
         <getter>
-          return this.getAttribute("direction");
+          this._mutationActor(this._mutationObserver.takeRecords());
+          return this._direction;
         </getter>
         <setter>
+          <![CDATA[
+          if (typeof val == 'string') {
+            val = val.toLowerCase();
+          } else {
+            return;
+          }
+          if (val != 'left' && val != 'right' && val != 'up' && val != 'down') {
+            val = 'left';
+          }
+
           this.setAttribute("direction", val);
+          ]]>
         </setter>
       </property>
 
       <property name="behavior" exposeToUntrustedContent="true">
         <getter>
           this._mutationActor(this._mutationObserver.takeRecords());
           return this._behavior;
         </getter>
         <setter>
-          this.setAttribute("behavior", val);
+          if (typeof val == 'string') {
+            val = val.toLowerCase();
+          }
+          if (val == "alternate" || val == "slide" || val == 'scroll') {
+            this.setAttribute("behavior", val);
+          }
         </setter>
       </property>
 
 
       <property name="loop" exposeToUntrustedContent="true">
         <getter>
           <![CDATA[
           this._mutationActor(this._mutationObserver.takeRecords());
-          var val = parseInt(this.getAttribute('loop'));
-    
-          if (val < -1 || isNaN(val))
-            return this._loop;
-
-          return val;
+          return this._loop;
           ]]>
         </getter>
         <setter>
-          this.setAttribute("loop", val);
+          <![CDATA[
+          var val = parseInt(val);
+          if (val == -1 || val > 0) {
+            this.setAttribute("loop", val);
+          }
+          ]]>
         </setter>
       </property>
 
 
       <property name="onstart" exposeToUntrustedContent="true">
         <getter>
           return this.getAttribute("onstart");
         </getter>
@@ -156,109 +183,117 @@
         onset="this.setAttribute('width', val);"
       />
 
       <method name="_set_scrollDelay">
         <parameter name="aValue"/>
         <body>
         <![CDATA[
           aValue = parseInt(aValue);
-          if (aValue <= 0 || isNaN(aValue) || aValue == null)
-            return false;
-
-          if (aValue < 60) {
-            if (this.trueSpeed == true)
+          if (aValue <= 0) {
+            return;
+          } else if (isNaN(aValue)) {
+            this._scrollDelay = 85;
+          } else if (aValue < 60) {
+            if (this.trueSpeed == true) {
               this._scrollDelay = aValue;
-            else
+            } else {
               this._scrollDelay = 60;
-          }
-          else {
+            }
+          } else {
             this._scrollDelay = aValue;
           }
-          return true;
         ]]>
         </body>
       </method>
 
       <method name="_set_scrollAmount">
         <parameter name="aValue"/>
         <body>
         <![CDATA[
           aValue = parseInt(aValue);
-          if (aValue < 0 || isNaN(aValue) || aValue == null)
-            return false;
-
-          this._scrollAmount = aValue;
-          return true;
+          if (isNaN(aValue)) {
+            this._scrollAmount = 6;
+          } else if (aValue < 0) {
+            return;
+          } else {
+            this._scrollAmount = aValue;
+          }
         ]]>
         </body>
       </method>
 
       <method name="_set_behavior">
         <parameter name="aValue"/>
         <body>
         <![CDATA[
-          if (typeof aValue == 'string')
+          if (typeof aValue == 'string') {
             aValue = aValue.toLowerCase();
-          if (aValue != 'alternate' && aValue != 'slide' && aValue != 'scroll')
-            return false;
-
-          this._behavior = aValue;
-          return true;
+          }
+          if (aValue != 'alternate' && aValue != 'slide' && aValue != 'scroll') {
+            this._behavior = 'scroll';
+          } else {
+            this._behavior = aValue;
+          }
         ]]>
         </body>
       </method>
 
       <method name="_set_direction">
         <parameter name="aValue"/>
         <body>
         <![CDATA[
-          if (typeof aValue == 'string')
+          if (typeof aValue == 'string') {
             aValue = aValue.toLowerCase();
-          if (aValue != 'left' && aValue != 'right' && aValue != 'up' && aValue != 'down')
-            return false;
+          }
+          if (aValue != 'left' && aValue != 'right' && aValue != 'up' && aValue != 'down') {
+            aValue = 'left';
+          }
 
-          if (aValue != this._direction)
+          if (aValue != this._direction) {
             this.startNewDirection = true;
+          }
           this._direction = aValue;
-          return true;
         ]]>
         </body>
       </method>
 
       <method name="_set_loop">
         <parameter name="aValue"/>
         <body>
           <![CDATA[
-          if (!aValue || isNaN(aValue))
-            return false;
-
-          if (aValue < -1)
+          var aValue = parseInt(aValue);
+          if (aValue == 0) {
+            return;
+          }
+          if (isNaN(aValue) || aValue <= -1) {
             aValue = -1;
-
+          }
           this._loop = aValue;
-          return true;
           ]]>
         </body>
       </method>
 
       <method name="_setEventListener">
         <parameter name="aName"/>
         <parameter name="aValue"/>
         <parameter name="aIgnoreNextCall"/>
         <body>
           <![CDATA[
-          if (this._ignoreNextCall)
+          if (this._ignoreNextCall) {
             return this._ignoreNextCall = false;
+          }
 
-          if (aIgnoreNextCall)
+          if (aIgnoreNextCall) {
             this._ignoreNextCall = true;
+          }
 
-          if (typeof this["_on" + aName] == 'function')
+          if (typeof this["_on" + aName] == 'function') {
             this.removeEventListener(aName, this["_on" + aName], false);
+          }
 
           switch (typeof aValue)
           {
             case "function":
               this["_on" + aName] = aValue;
               this.addEventListener(aName, this["_on" + aName], false);
             break;
 
@@ -342,52 +377,56 @@
 
             var corrvalue = 0;
 
             switch (this._direction)
             {
               case "up":
                 var height = document.defaultView.getComputedStyle(this, "").height;
                 this.outerDiv.style.height = height;
-                if (this.originalHeight > this.outerDiv.offsetHeight)
+                if (this.originalHeight > this.outerDiv.offsetHeight) {
                     corrvalue = this.originalHeight - this.outerDiv.offsetHeight;
+                }
                 this.innerDiv.style.padding = height + " 0";
                 this.dirsign = 1;
                 this.startAt = (this._behavior == 'alternate') ? (this.originalHeight - corrvalue) : 0;
                 this.stopAt  = (this._behavior == 'alternate' || this._behavior == 'slide') ? 
                                 (parseInt(height) + corrvalue) : (this.originalHeight + parseInt(height));
               break;
 
               case "down":
                 var height = document.defaultView.getComputedStyle(this, "").height;
                 this.outerDiv.style.height = height;
-                if (this.originalHeight > this.outerDiv.offsetHeight)
+                if (this.originalHeight > this.outerDiv.offsetHeight) {
                     corrvalue = this.originalHeight - this.outerDiv.offsetHeight;
+                }
                 this.innerDiv.style.padding = height + " 0";
                 this.dirsign = -1;
                 this.startAt  = (this._behavior == 'alternate') ?
                                 (parseInt(height) + corrvalue) : (this.originalHeight + parseInt(height));
                 this.stopAt = (this._behavior == 'alternate' || this._behavior == 'slide') ? 
                               (this.originalHeight - corrvalue) : 0;
               break;
 
               case "right":
-                if (this.innerDiv.offsetWidth > this.outerDiv.offsetWidth)
+                if (this.innerDiv.offsetWidth > this.outerDiv.offsetWidth) {
                     corrvalue = this.innerDiv.offsetWidth - this.outerDiv.offsetWidth;
+                }
                 this.dirsign = -1;
                 this.stopAt  = (this._behavior == 'alternate' || this._behavior == 'slide') ? 
                                (this.innerDiv.offsetWidth - corrvalue) : 0;
                 this.startAt = this.outerDiv.offsetWidth + ((this._behavior == 'alternate') ? 
                                corrvalue : (this.innerDiv.offsetWidth + this.stopAt));   
               break;
 
               case "left":
               default:
-                if (this.innerDiv.offsetWidth > this.outerDiv.offsetWidth)
+                if (this.innerDiv.offsetWidth > this.outerDiv.offsetWidth) {
                     corrvalue = this.innerDiv.offsetWidth - this.outerDiv.offsetWidth;
+                }
                 this.dirsign = 1;
                 this.startAt = (this._behavior == 'alternate') ? (this.innerDiv.offsetWidth - corrvalue) : 0;
                 this.stopAt  = this.outerDiv.offsetWidth + 
                                ((this._behavior == 'alternate' || this._behavior == 'slide') ? 
                                corrvalue : (this.innerDiv.offsetWidth + this.startAt));
             }
 
             if (aResetPosition) {
@@ -407,59 +446,65 @@
                 // lets start afresh
                 this.startNewDirection = true;
 
                 // swap direction
                 const swap = {left: "right", down: "up", up: "down", right: "left"};
                 this._direction = swap[this._direction];
                 this.newPosition = this.stopAt;
 
-                if ((this._direction == "up") || (this._direction == "down"))
+                if ((this._direction == "up") || (this._direction == "down")) {
                   this.outerDiv.scrollTop = this.newPosition;
-                else
+                } else {
                   this.outerDiv.scrollLeft = this.newPosition;
+                }
 
-                if (this._loop != 1)
+                if (this._loop != 1) {
                   this._fireEvent("bounce", false, true);
+                }
               break;
 
               case 'slide':
-                if (this._loop > 1)
+                if (this._loop > 1) {
                   this.newPosition = this.startAt;
+                }
               break;
 
               default:
                 this.newPosition = this.startAt;
 
-                if ((this._direction == "up") || (this._direction == "down"))
+                if ((this._direction == "up") || (this._direction == "down")) {
                   this.outerDiv.scrollTop = this.newPosition;
-                else
+                } else {
                   this.outerDiv.scrollLeft = this.newPosition;
+                }
 
                 //dispatch start event, even when this._loop == 1, comp. with IE6
                 this._fireEvent("start", false, false);
             }
 
-            if (this._loop > 1)
+            if (this._loop > 1) {
               this._loop--;
-            else if (this._loop == 1) {
-              if ((this._direction == "up") || (this._direction == "down"))
+            } else if (this._loop == 1) {
+              if ((this._direction == "up") || (this._direction == "down")) {
                 this.outerDiv.scrollTop = this.stopAt;
-              else
+              } else {
                 this.outerDiv.scrollLeft = this.stopAt;
+              }
               this.stop();
               this._fireEvent("finish", false, true);
               return;
             }
           }
           else {
-            if ((this._direction == "up") || (this._direction == "down"))
+            if ((this._direction == "up") || (this._direction == "down")) {
               this.outerDiv.scrollTop = this.newPosition;
-            else
+            } else {
               this.outerDiv.scrollLeft = this.newPosition;
+            }
           }
 
           var myThis = this;
           var lambda = function myTimeOutFunction(){myThis._doMove(false);}
           this.runId = window.setTimeout(lambda, this._scrollDelay);
         ]]>
         </body>
       </method>
@@ -503,63 +548,49 @@
             var attrName = mutation.attributeName.toLowerCase();
             var oldValue = mutation.oldValue;
             var target = mutation.target;
             var newValue = target.getAttribute(attrName);
 
             if (oldValue != newValue) {
               switch (attrName) {
                 case "loop":
-                  if (!target._set_loop(newValue)) {
-                    if (!newValue) {
-                      target._loop = -1;
-                      if (target.runId == 0)
-                        target.start();
-                    }
+                  target._set_loop(newValue);
+                  if (target.rundId == 0) {
+                    target.start();
                   }
-                  if (target.rundId == 0)
-                    target.start();
                   break;
                 case "scrollamount":
-                  if (!newValue)
-                    target._scrollAmount = 6;
-                  else
-                    target._set_scrollAmount(newValue);
+                  target._set_scrollAmount(newValue);
                   break;
                 case "scrolldelay":
-                  if (!newValue)
-                    target._scrollDelay = 85;
-                  else
-                    target._set_scrollDelay(newValue);
+                  target._set_scrollDelay(newValue);
                   target.stop();
                   target.start();
                   break;
                 case "truespeed":
                   //needed to update target._scrollDelay
                   var myThis = target;
                   var lambda = function() {myThis._set_scrollDelay(myThis.getAttribute('scrolldelay'));}
                   window.setTimeout(lambda, 0);
                   break;
                 case "behavior":
-                  if (!newValue)
-                    target._behavior = "scroll";
-                  else
-                    target._set_behavior(newValue);
+                  target._set_behavior(newValue);
                   target.startNewDirection = true;
                   if ((oldValue == "slide" && target.newPosition == target.stopAt) ||
                       newValue == "alternate" || newValue == "slide") {
                     target.stop();
                     target._doMove(true);
                   }
                   break;
                 case "direction":
-                  if (!newValue)
-                    target._direction = "left";
-                  else
-                    target._set_direction(newValue);
+                  if (!newValue) {
+                    newValue = "left";
+                  }
+                  target._set_direction(newValue);
                   break;
                 case "width":
                 case "height":
                   target.startNewDirection = true;
                   break;
                 case "onstart":
                   target._setEventListener("start", newValue);
                   break;
@@ -602,24 +633,28 @@
           this._set_scrollAmount(this.getAttribute('scrollamount'));
           this._set_loop(this.getAttribute('loop'));
           this._setEventListener("start", this.getAttribute("onstart"));
           this._setEventListener("finish", this.getAttribute("onfinish"));
           this._setEventListener("bounce", this.getAttribute("onbounce"));
           this.startNewDirection = true;
 
           this._mutationObserver = new MutationObserver(this._mutationActor);
-          this._mutationObserver.observe(this, { attributes: true, attributeOldValue: true });
+          this._mutationObserver.observe(this, { attributes: true,
+            attributeOldValue: true,
+            attributeFilter: ['loop', 'scrollamount', 'scrolldelay', '', 'truespeed', 'behavior',
+              'direction', 'width', 'height', 'onstart', 'onfinish', 'onbounce'] });
 
           // init needs to be run after the page has loaded in order to calculate
           // the correct height/width
-          if (document.readyState == "complete")
+          if (document.readyState == "complete") {
             lambda();
-          else
+          } else {
             window.addEventListener("load", lambda, false);
+          }
         ]]>
       </constructor>
     </implementation>
 
   </binding>
 
   <binding id="marquee-horizontal" bindToUntrustedContent="true"
            extends="chrome://xbl-marquee/content/xbl-marquee.xml#marquee"