658776 - Search API for JS dialog (accessible tree viewer)

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

the idea is to add function to allow to search accessible complying criteria in the accessible tree (expand tree tree and highlight them)

API is:

tree.search(aSubtreeRoot, aFliterFunc);
filter function takes traversed accessible and return true/false depending on the accessible complies criteria.

tree.clearSearch();

for example
function filterFunc(acc)
{
  return acc.name == 'Here's your next car';
}
tree.search(accessible, filterFunc);

highlights all accessibles in the tree having the given name.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch2

updated version

Comment 2

[Colby Russell :crussell (no longer Mozilla-ing)]

Comment on attachment 595657 [details] [diff] [review]
patch2

> +inAccTreeView.prototype.getRowProperties =
> +function getRowProperties(aRowIdx, aProperties)
> +{
> +  var node = this.rowToNode(aRowIdx);
> +  if (node && node.highlighted) {
> +    let atom = this.createAtom("highlight");
> +    aProperties.AppendElement(atom);
> +  }
> +}
> +
> +inAccTreeView.prototype.getCellProperties =
> +function getCellProperties(aRowIdx, aCol, aProperties)
> +{
> +  var node = this.rowToNode(aRowIdx);
> +  if (node && node.highlighted) {
> +    let atom = this.createAtom("highlight");
> +    aProperties.AppendElement(atom);
> +  }
> +}
> +

Factor this into a common routine? XXX

> + * Expand the tree and highlight accessibles in the given subtree that comply
> + * to filter function.
> + *
> + * @param aRoot        [in] the root accessible of subtree to search in
> + * @param aFilterFunc  [in] a function that returns true if the passed
> + *                       accessible complies to search criteria.
> + */

The other inline documentation in this file looks something like:
/**
 * Return row index of the last node that is a descendant of the given node.
 * If there is no required node then return the given row.
 *
 * @param aNode - tree node for that last descedant is searched.
 * @param aRow - row index of the given tree node.
 */

[inAccTreeView.prototype.getAccessible is the only other function that doesn't do it like this.  It would be nice if it did.]

> +inAccTreeView.prototype.search =
> +function search(aRoot, aFilterFunc)
> +{
> +  QIAccessNode(aRoot);
> +  if (aFilterFunc(aRoot)) {
> +    let chain = [];
> +    let parent = aRoot;
> +    do {
> +      chain.push(parent);
> +      if (parent == aRoot.document)
> +        break;
> +
> +      parent = parent.parent;
> +    } while (parent);
> +
> +    var current = chain.pop();

You use |var| here and |let| above for the same scope, but |current| is never used outside of this block, so there's no special need for the difference.  Please either change |var current| to |let| or change both |let chain| and |let parent| to use |var|.  (I prefer the first one.)

> diff --git a/jar.mn b/jar.mn
> --- a/jar.mn
> +++ b/jar.mn
> @@ -136,16 +136,17 @@ inspector.jar:
>    skin/classic/inspector/inspectorWindow.css                            (resources/skin/classic/inspectorWindow.css)
>    skin/classic/inspector/titledSplitter.css                             (resources/skin/classic/titledSplitter.css)
>    skin/classic/inspector/titledsplitter-close.gif                       (resources/skin/classic/titledsplitter-close.gif)
>    skin/classic/inspector/panelset.css                                   (resources/skin/classic/panelset.css)
>    skin/classic/inspector/sidebar.css                                    (resources/skin/classic/sidebar.css)
>    skin/classic/inspector/iconViewerList-dis.gif                         (resources/skin/classic/iconViewerList-dis.gif)
>    skin/classic/inspector/viewers/accessibleEvent/accessibleEvent.css    (resources/skin/classic/viewers/accessibleEvent/accessibleEvent.css)
>    skin/classic/inspector/viewers/accessibleEvents/accessibleEvents.css  (resources/skin/classic/viewers/accessibleEvents/accessibleEvents.css)
> +  skin/classic/inspector/viewers/accessibleTree/accessibleTree.css      (resources/skin/classic/viewers/accessibleTree/accessibleTree.css)
>    skin/classic/inspector/viewers/boxModel/boxModel.css                  (resources/skin/classic/viewers/boxModel/boxModel.css)
>    skin/classic/inspector/viewers/dom/columnsDialog.css                  (resources/skin/classic/viewers/dom/columnsDialog.css)
>    skin/classic/inspector/viewers/dom/dom.css                            (resources/skin/classic/viewers/dom/dom.css)
>    skin/classic/inspector/viewers/dom/findDialog.css                     (resources/skin/classic/viewers/dom/findDialog.css)
>    skin/classic/inspector/viewers/domNode/domNode.css                    (resources/skin/classic/viewers/domNode/domNode.css)
>    skin/classic/inspector/viewers/styleRules/styleRules.css              (resources/skin/classic/viewers/styleRules/styleRules.css)
>    skin/classic/inspector/viewers/xblBindings/xblBindings.css            (resources/skin/classic/viewers/xblBindings/xblBindings.css)
>  % skin inspector modern/1.0 %skin/modern/inspector/
> @@ -163,15 +164,16 @@ inspector.jar:
>    skin/modern/inspector/titledsplitter-close.gif                        (resources/skin/modern/titledsplitter-close.gif)
>    skin/modern/inspector/panelset.css                                    (resources/skin/modern/panelset.css)
>    skin/modern/inspector/sidebar.css                                     (resources/skin/modern/sidebar.css)
>    skin/modern/inspector/iconViewerMenu.gif                              (resources/skin/modern/iconViewerMenu.gif)
>    skin/modern/inspector/iconViewerList.gif                              (resources/skin/modern/iconViewerList.gif)
>    skin/modern/inspector/iconViewerList-dis.gif                          (resources/skin/modern/iconViewerList-dis.gif)
>    skin/modern/inspector/viewers/accessibleEvent/accessibleEvent.css     (resources/skin/modern/viewers/accessibleEvent/accessibleEvent.css)
>    skin/modern/inspector/viewers/accessibleEvents/accessibleEvents.css   (resources/skin/modern/viewers/accessibleEvents/accessibleEvents.css)
> +  skin/modern/inspector/viewers/accessibleTree/accessibleTree.css       (resources/skin/modern/viewers/accessibleTree/accessibleTree.css)
>    skin/modern/inspector/viewers/boxModel/boxModel.css                   (resources/skin/modern/viewers/boxModel/boxModel.css)
>    skin/modern/inspector/viewers/dom/columnsDialog.css                   (resources/skin/modern/viewers/dom/columnsDialog.css)
>    skin/modern/inspector/viewers/dom/dom.css                             (resources/skin/modern/viewers/dom/dom.css)
>    skin/modern/inspector/viewers/dom/findDialog.css                      (resources/skin/modern/viewers/dom/findDialog.css)
>    skin/modern/inspector/viewers/domNode/domNode.css                     (resources/skin/modern/viewers/domNode/domNode.css)
>    skin/modern/inspector/viewers/styleRules/styleRules.css               (resources/skin/modern/viewers/styleRules/styleRules.css)
>    skin/modern/inspector/viewers/xblBindings/xblBindings.css             (resources/skin/modern/viewers/xblBindings/xblBindings.css)
…
> diff --git a/resources/content/viewers/accessibleTree/accessibleTree.xul b/resources/content/viewers/accessibleTree/accessibleTree.xul
> --- a/resources/content/viewers/accessibleTree/accessibleTree.xul
> +++ b/resources/content/viewers/accessibleTree/accessibleTree.xul
> @@ -37,17 +37,18 @@
>     -
>     - ***** END LICENSE BLOCK ***** -->
>  
>  <!DOCTYPE page [
>    <!ENTITY % dtd1 SYSTEM "chrome://inspector/locale/inspector.dtd"> %dtd1;
>    <!ENTITY % dtd2 SYSTEM "chrome://inspector/locale/viewers/accessibleTree.dtd"> %dtd2;
>  ]>
>  
> -<?xml-stylesheet href="chrome://inspector/skin/"?>
> +<?xml-stylesheet href="chrome://inspector/skin/viewers/accessibleTree/accessibleTree.css"?>
> +
>  <?xul-overlay href="chrome://inspector/content/editingOverlay.xul"?>
>  
>  <page id="winAccessibleTree"
>        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
>  
>    <script type="application/javascript"
>            src="chrome://inspector/content/hooks.js"/>
>    <script type="application/javascript"
> @ …
> diff --git a/resources/skin/classic/viewers/accessibleEvent/accessibleEvent.css b/resources/skin/classic/viewers/accessibleTree/accessibleTree.css
> copy from resources/skin/classic/viewers/accessibleEvent/accessibleEvent.css
> copy to resources/skin/classic/viewers/accessibleTree/accessibleTree.css
> --- a/resources/skin/classic/viewers/accessibleEvent/accessibleEvent.css
> +++ b/resources/skin/classic/viewers/accessibleTree/accessibleTree.css
> @@ -10,17 +10,17 @@
>   * 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 DOM Inspector.
>   *
>   * The Initial Developer of the Original Code is
>   * Mozilla Foundation.
> - * Portions created by the Initial Developer are Copyright (C) 2010
> + * Portions created by the Initial Developer are Copyright (C) 2011
>   * the Initial Developer. All Rights Reserved.
>   *
>   * Contributor(s):
>   *  Alexander Surkov <[e-mail]> (original author)
>   *
>   * Alternatively, the contents of this file may be used under the terms of
>   * either 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"),
> diff --git a/resources/skin/modern/viewers/accessibleEvent/accessibleEvent.css b/resources/skin/modern/viewers/accessibleTree/accessibleTree.css
> copy from resources/skin/modern/viewers/accessibleEvent/accessibleEvent.css
> copy to resources/skin/modern/viewers/accessibleTree/accessibleTree.css
> --- a/resources/skin/modern/viewers/accessibleEvent/accessibleEvent.css
> +++ b/resources/skin/modern/viewers/accessibleTree/accessibleTree.css
> @@ -10,17 +10,17 @@
>   * 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 DOM Inspector.
>   *
>   * The Initial Developer of the Original Code is
>   * Mozilla Foundation.
> - * Portions created by the Initial Developer are Copyright (C) 2010
> + * Portions created by the Initial Developer are Copyright (C) 2011
>   * the Initial Developer. All Rights Reserved.
>   *
>   * Contributor(s):
>   *  Alexander Surkov <[e-mail]> (original author)
>   *
>   * Alternatively, the contents of this file may be used under the terms of
>   * either 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"),

[I don't think this is necessary, since the file was copied without changes, it's not 2011, and all these license blocks are going away with the MPL 2 relicensing (bug 716554).]

And without changes, accessibleTree.xul can use accessibleEvent.css without creating separate files.

Having said that, it doesn't look like the #handlerOutput rule from accessibleEvent.css is needed here, but I might be wrong.  If I'm not, and we keep accessibleTree.css, the rule should be removed there.  And in that case, the highlight rules from accessibleEvent.css can be removed as well, and it can just @import accessibleTree.css.]

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 4

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/dom-inspector/rev/70cfd118a92a