Bug 1184989 - Prevent flipping preference will scroll the page as wel…

…l. r=jaws

MozReview-Commit-ID: 5j5FN8aFDlX

browser/components/preferences/in-content/search.js

@@ -223,6 +223,8 @@ var gSearchPane = {
       let newValue = !gEngineView._engineStore.engines[index].shown;
       gEngineView.setCellValue(index, tree.columns.getFirstColumn(),
                                newValue.toString());
+      // Prevent page from scrolling on the space key.
+      aEvent.preventDefault();
     }
     else {
       let isMac = Services.appinfo.OS == "Darwin";

browser/components/preferences/in-content/sync.js

@@ -559,12 +559,16 @@ var gSyncPane = {
           replaceQueryString: true
         });
       });
+      // Prevent page from scrolling on the space key.
+      event.preventDefault();
     }
   },
 
   openManageFirefoxAccount: function(event) {
     if (this.clickOrSpaceOrEnterPressed(event)) {
       this.manageFirefoxAccount();
+      // Prevent page from scrolling on the space key.
+      event.preventDefault();
     }
   },
 
@@ -667,4 +671,3 @@ var gSyncPane = {
     textbox.value = value;
   },
 };
-

browser/components/preferences/in-content/tests/browser.ini

@@ -12,6 +12,9 @@ support-files =
 [browser_bug795764_cachedisabled.js]
 [browser_bug1018066_resetScrollPosition.js]
 [browser_bug1020245_openPreferences_to_paneContent.js]
+[browser_bug1184989_prevent_scrolling_when_preferences_flipped.js]
+support-files =
+  browser_bug1184989_prevent_scrolling_when_preferences_flipped.xul
 [browser_change_app_handler.js]
 skip-if = os != "win" # This test tests the windows-specific app selection dialog, so can't run on non-Windows
 [browser_connection.js]

browser/components/preferences/in-content/tests/browser_bug1184989_prevent_scrolling_when_preferences_flipped.js

@@ -0,0 +1,92 @@
+const ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
+
+add_task(function* () {
+  waitForExplicitFinish();
+
+  const tabURL = getRootDirectory(gTestPath) + "browser_bug1184989_prevent_scrolling_when_preferences_flipped.xul";
+
+  yield BrowserTestUtils.withNewTab({ gBrowser, url: tabURL }, function* (browser) {
+    let doc = browser.contentDocument;
+    let container = doc.getElementById("container");
+
+    // Test button
+    let button = doc.getElementById("button");
+    button.focus();
+    EventUtils.synthesizeKey(" ", {});
+    yield checkPageScrolling(container, "button");
+
+    // Test checkbox
+    let checkbox = doc.getElementById("checkbox");
+    checkbox.focus();
+    EventUtils.synthesizeKey(" ", {});
+    ok(checkbox.checked, "Checkbox is checked");
+    yield checkPageScrolling(container, "checkbox");
+
+    // Test listbox
+    let listbox = doc.getElementById("listbox");
+    let listitem = doc.getElementById("listitem");
+    listbox.focus();
+    EventUtils.synthesizeKey(" ", {});
+    ok(listitem.selected, "Listitem is selected");
+    yield checkPageScrolling(container, "listbox");
+
+    // Test radio
+    let radiogroup = doc.getElementById("radiogroup");
+    radiogroup.focus();
+    EventUtils.synthesizeKey(" ", {});
+    yield checkPageScrolling(container, "radio");
+  });
+
+  yield BrowserTestUtils.withNewTab({ gBrowser, url: "about:preferences#search" }, function* (browser) {
+    let doc = browser.contentDocument;
+    let container = doc.getElementsByClassName("main-content")[0];
+
+    // Test search
+    let engineList = doc.getElementById("engineList");
+    engineList.focus();
+    EventUtils.synthesizeKey(" ", {});
+    is(engineList.view.selection.currentIndex, 0, "Search engineList is selected");
+    EventUtils.synthesizeKey(" ", {});
+    yield checkPageScrolling(container, "search engineList");
+  });
+
+  // Test session restore
+  const CRASH_URL = "about:mozilla";
+  const CRASH_FAVICON = "chrome://branding/content/icon32.png";
+  const CRASH_SHENTRY = {url: CRASH_URL};
+  const CRASH_TAB = {entries: [CRASH_SHENTRY], image: CRASH_FAVICON};
+  const CRASH_STATE = {windows: [{tabs: [CRASH_TAB]}]};
+
+  const TAB_URL = "about:sessionrestore";
+  const TAB_FORMDATA = {url: TAB_URL, id: {sessionData: CRASH_STATE}};
+  const TAB_SHENTRY = {url: TAB_URL};
+  const TAB_STATE = {entries: [TAB_SHENTRY], formdata: TAB_FORMDATA};
+
+  let tab = gBrowser.selectedTab = gBrowser.addTab("about:blank");
+
+  // Fake a post-crash tab
+  ss.setTabState(tab, JSON.stringify(TAB_STATE));
+
+  yield BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  let doc = tab.linkedBrowser.contentDocument;
+
+  // Make body scrollable
+  doc.body.style.height = (doc.body.clientHeight + 100) + "px";
+
+  let tabList = doc.getElementById("tabList");
+  tabList.focus();
+  EventUtils.synthesizeKey(" ", {});
+  yield checkPageScrolling(doc.documentElement, "session restore");
+
+  gBrowser.removeCurrentTab();
+  finish();
+});
+
+function checkPageScrolling(container, type) {
+  return new Promise(resolve => {
+    setTimeout(() => {
+      is(container.scrollTop, 0, "Page should not scroll when " + type + " flipped");
+      resolve();
+    }, 0);
+  });
+}

browser/components/preferences/in-content/tests/browser_bug1184989_prevent_scrolling_when_preferences_flipped.xul

@@ -0,0 +1,33 @@
+<?xml version="1.0"?>
+<!--
+  XUL Widget Test for Bug 1184989
+  -->
+<page title="Bug 1184989 Test"
+      xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<vbox id="container" style="height: 200px; overflow: auto;">
+  <vbox style="height: 500px;">
+    <hbox>
+      <button id="button" label="button" />
+    </hbox>
+
+    <hbox>
+      <checkbox id="checkbox" label="checkbox" />
+    </hbox>
+
+    <hbox style="height: 50px;">
+      <listbox id="listbox">
+        <listitem id="listitem" label="listitem" />
+        <listitem label="listitem" />
+      </listbox>
+    </hbox>
+
+    <hbox>
+      <radiogroup id="radiogroup">
+        <radio id="radio" label="radio" />
+      </radiogroup>
+    </hbox>
+  </vbox>
+</vbox>
+
+</page>

browser/components/sessionstore/content/aboutSessionRestore.js

@@ -205,6 +205,8 @@ function onListKeyDown(aEvent) {
   {
   case KeyEvent.DOM_VK_SPACE:
     toggleRowChecked(document.getElementById("tabList").currentIndex);
+    // Prevent page from scrolling on the space key.
+    aEvent.preventDefault();
     break;
   case KeyEvent.DOM_VK_RETURN:
     var ix = document.getElementById("tabList").currentIndex;

toolkit/content/widgets/button.xml

@@ -143,7 +143,13 @@
            and then they would see the incorrect value of checked. Additionally
            a command attribute would redirect the command events anyway.-->
       <handler event="click" button="0" action="this._handleClick();"/>
-      <handler event="keypress" key=" " action="this._handleClick();"/>
+      <handler event="keypress" key=" ">
+      <![CDATA[
+        this._handleClick();
+        // Prevent page from scrolling on the space key.
+        event.preventDefault();
+      ]]>
+      </handler>
 
       <handler event="keypress">
       <![CDATA[
@@ -246,7 +252,13 @@
 
     <handlers>
       <handler event="keypress" keycode="VK_RETURN" action="this.open = true;"/>
-      <handler event="keypress" key=" " action="this.open = true;"/>
+      <handler event="keypress" key=" ">
+      <![CDATA[
+        this.open = true;
+        // Prevent page from scrolling on the space key.
+        event.preventDefault();
+      ]]>
+      </handler>
     </handlers>
   </binding>
 
@@ -337,8 +349,11 @@
           this.open = true;
       </handler>
       <handler event="keypress" key=" ">
-        if (event.originalTarget == this)
+        if (event.originalTarget == this) {
           this.open = true;
+          // Prevent page from scrolling on the space key.
+          event.preventDefault();
+        }
       </handler>
     </handlers>
   </binding>

toolkit/content/widgets/checkbox.xml

@@ -24,7 +24,7 @@
         <xul:label class="checkbox-label" xbl:inherits="xbl:text=label,accesskey,crop" flex="1"/>
       </xul:hbox>
     </content>
-    
+
     <implementation implements="nsIDOMXULCheckboxElement">
       <method name="setChecked">
         <parameter name="aValue"/>
@@ -44,24 +44,26 @@
         ]]>
         </body>
       </method>
-      
+
       <!-- public implementation -->
       <property name="checked"    onset="return this.setChecked(val);"
                                   onget="return this.getAttribute('checked') == 'true';"/>
     </implementation>
-    
+
     <handlers>
       <!-- While it would seem we could do this by handling oncommand, we need can't
            because any external oncommand handlers might get called before ours, and
-           then they would see the incorrect value of checked. -->           
+           then they would see the incorrect value of checked. -->
       <handler event="click" button="0" action="if (!this.disabled) this.checked = !this.checked;"/>
       <handler event="keypress" key=" ">
         <![CDATA[
           this.checked = !this.checked;
+          // Prevent page from scrolling on the space key.
+          event.preventDefault();
         ]]>
       </handler>
     </handlers>
-  </binding> 
+  </binding>
 
   <binding id="checkbox-with-spacing"
            extends="chrome://global/content/bindings/checkbox.xml#checkbox">

toolkit/content/widgets/listbox.xml

@@ -822,11 +822,16 @@
       <handler event="keypress" key=" " phase="target">
         <![CDATA[
           if (this.currentItem) {
-            if (this.currentItem.getAttribute("type") != "checkbox")
+            if (this.currentItem.getAttribute("type") != "checkbox") {
               this.addItemToSelection(this.currentItem);
+              // Prevent page from scrolling on the space key.
+              event.preventDefault();
+            }
             else if (!this.currentItem.disabled) {
               this.currentItem.checked = !this.currentItem.checked;
               this.currentItem.doCommand();
+              // Prevent page from scrolling on the space key.
+              event.preventDefault();
             }
           }
         ]]>

toolkit/content/widgets/radio.xml

@@ -370,8 +370,10 @@
            is done solely through the arrow keys and not the tab button. Tab
            takes you to the next widget in the tab order -->
       <handler event="keypress" key=" " phase="target">
-         this.selectedItem = this.focusedItem;
-         this.selectedItem.doCommand();
+        this.selectedItem = this.focusedItem;
+        this.selectedItem.doCommand();
+        // Prevent page from scrolling on the space key.
+        event.preventDefault();
       </handler>
       <handler event="keypress" keycode="VK_UP" phase="target">
         this.checkAdjacentElement(false);