dom-ui settings: Enable searching for sub-pages...

chrome/browser/resources/options/options_page.js

Index: chrome/browser/resources/options/options_page.js
===================================================================
--- chrome/browser/resources/options/options_page.js	(revision 70013)
+++ chrome/browser/resources/options/options_page.js	(working copy)
@@ -48,9 +48,26 @@
    */
   OptionsPage.showPageByName = function(pageName) {
     var targetPage = this.registeredPages[pageName];
+
+    // Determine if the root page is 'sticky', meaning that it
+    // shouldn't change when showing a sub-page.  This can happen for special
+    // pages like Search.
+    var rootPage = null;
+    for (var name in this.registeredPages) {
+      var page = this.registeredPages[name];
+      if (page.visible && !page.parentPage) {
+        rootPage = page;
+        break;
+      }
+    }
+    var isRootPageLocked =
+        (rootPage && rootPage.sticky && targetPage.parentPage);

[arv (Not doing code reviews), 2010/12/23 20:38:22]

useless parentheses

[csilv, 2010/12/23 21:46:07]

Done.

+
     // Notify pages if they will be hidden.
     for (var name in this.registeredPages) {
       var page = this.registeredPages[name];
+      if (!page.parentPage && isRootPageLocked)
+        continue;
       if (page.willHidePage && name != pageName &&
           !page.isAncestorOfPage(targetPage))
         page.willHidePage();
@@ -59,6 +76,8 @@
     // Update visibilities to show only the hierarchy of the target page.
     for (var name in this.registeredPages) {
       var page = this.registeredPages[name];
+      if (!page.parentPage && isRootPageLocked)
+        continue;
       page.visible = name == pageName ||
           (document.documentElement.getAttribute('hide-menu') != 'true' &&
            page.isAncestorOfPage(targetPage));
@@ -67,7 +86,10 @@
     // Notify pages if they were shown.
     for (var name in this.registeredPages) {
       var page = this.registeredPages[name];
-      if (name == pageName && page.didShowPage)
+      if (!page.parentPage && isRootPageLocked)
+        continue;
+      if (page.didShowPage && (name == pageName ||
+          page.isAncestorOfPage(targetPage)))
         page.didShowPage();
     }
   };
@@ -215,12 +237,38 @@
   };
 
   /**
-   * Registers a new Sub tab page.
-   * @param {OptionsPage} page Page to register.
+   * Find an enclosing section for an element if it exists.
+   * @param {Element} element Element to search.
+   * @return {OptionPage} The section element, or null.
+   * @private
    */
-  OptionsPage.registerSubPage = function(subPage, parentPage) {
+  OptionsPage.findSectionForNode_ = function(node) {
+    while (node = node.parentNode) {

[arv (Not doing code reviews), 2010/12/23 20:38:22]

node.parentElement since you don't care about non elements.

[csilv, 2010/12/23 21:46:07]

parentElement?  I can't find any references to that other than mention that it's
supported in IE.

Nevertheless, it's probably unnecessary anyway.  parentNode is always going to
return an element or null.  So my extra checking of the nodeType is unnecessary.
 Will simplify.

+      if (node.nodeType == document.ELEMENT_NODE &&

[arv (Not doing code reviews), 2010/12/23 20:38:22]

Node.Element_NODE

... if you use parentNode you can skip the nodeType check

[csilv, 2010/12/23 21:46:07]

Done.

+          node.nodeName.toLowerCase() == 'section')

[arv (Not doing code reviews), 2010/12/23 20:38:22]

Use tagName?

I would skip the  toLowerCase and compare with 'SECTION' since tagName is always
upper case in HTML

[csilv, 2010/12/23 21:46:07]

Done.

+        return node;
+    }
+    return null;
+  };
+
+  /**
+   * Registers a new Sub-page.
+   * @param {OptionsPage} subPage Sub-page to register.
+   * @param {OptionsPage} parentPage Associated parent page for this page.
+   * @param {Array} associatedControls Array of control elements that lead to
+   *   this sub-page.  The first item is typically a button in a root-level

[arv (Not doing code reviews), 2010/12/23 20:38:22]

indent +2

[csilv, 2010/12/23 21:46:07]

Done.

+   *   page.  There may be additional buttons for nested sub-pages.
+   */
+  OptionsPage.registerSubPage = function(subPage,
+                                         parentPage,
+                                         associatedControls) {

[arv (Not doing code reviews), 2010/12/23 20:38:22]

Can you make this optional?

[csilv, 2010/12/23 21:46:07]

Done.

     this.registeredPages[subPage.name] = subPage;
     subPage.parentPage = parentPage;
+    subPage.associatedControls = associatedControls;
+    if (associatedControls && associatedControls.length) {
+      subPage.associatedSection =
+          this.findSectionForNode_(associatedControls[0]);
+    }
     subPage.tab = undefined;
     subPage.initializePage();
   };
@@ -228,10 +276,16 @@
   /**
    * Registers a new Overlay page.
    * @param {OptionsPage} page Page to register, must be a class derived from
-   * OptionsPage.
+   * @param {Array} associatedControls Array of control elements associated with
+   *   this page.
    */
-  OptionsPage.registerOverlay = function(page) {
+  OptionsPage.registerOverlay = function(page,
+                                         associatedControls) {

[arv (Not doing code reviews), 2010/12/23 20:38:22]

here too

[csilv, 2010/12/23 21:46:07]

Done.

     this.registeredOverlayPages[page.name] = page;
+    page.associatedControls = associatedControls;
+    if (associatedControls && associatedControls.length) {
+      page.associatedSection = this.findSectionForNode_(associatedControls[0]);
+    }
     page.tab = undefined;
     page.isOverlay = true;
     page.initializePage();
@@ -321,6 +375,20 @@
     parentPage: null,
 
     /**
+     * The section on the parent page that is associated with this page.
+     * Can be null.
+     * @type {Element}
+     */
+    associatedSection: null,
+
+    /**
+     * An array of controls that are associated with this page.  The first
+     * control should be located on a top-level page.
+     * @type {OptionsPage}
+     */
+    associatedControls: null,
+
+    /**
      * Initializes page content.
      */
     initializePage: function() {},
@@ -420,6 +488,15 @@
     },
 
     /**
+     * Checks whether the page is considered 'sticky', such that it will
+     * remain a top-level page even if sub-pages change.
+     * @return {boolean} True if this page is sticky.

[arv (Not doing code reviews), 2010/12/23 20:38:22]

I've been using @type for getters and setters since they look like properties
and not functions... I don't know the official stance on this

[csilv, 2010/12/23 21:46:07]

Done.

+     */
+    get sticky() {
+      return false;
+    },
+
+    /**
      * Checks whether this page is an ancestor of the given page in terms of
      * subpage nesting.
      * @param {OptionsPage} page