Closed Bug 322610 Opened 19 years ago Closed 17 years ago

Page Info needs a hook in grabAll() for extensions/overlays to use

Categories

(Firefox :: Page Info Window, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: db48x, Assigned: florian)

Details

Attachments

(1 file, 1 obsolete file)

Otherwise they all have to reimplement the whole process of running through all the nodes in the page for themselves.

I've been meaning to post this patch for quite a while.
Attached patch 322610-1.diff (obsolete) — Splinter Review
Attachment #207770 - Flags: review?
Attachment #207770 - Flags: review? → review?(mconnor)
Attachment #207770 - Flags: review?(mconnor) → review+
Attached patch 322610-2.diffSplinter Review
actually, this patch is more useful in the use-case we've got at the moment (an extension from Florian Queze)
Attachment #207770 - Attachment is obsolete: true
Attachment #208790 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #208790 - Flags: review?
Attachment #208790 - Flags: review? → review?(mconnor)
Comment on attachment 208790 [details] [diff] [review]
322610-2.diff

>+// These are called once for each subframe of the target docuemnt The frame is
>+// passed as an argument.
"target document. The"

>+// These functions are called once when all the elements in all of the target
>+// document (and all of it's subframes, if any) have been processed
"of its subframes"; what's the use case btw?

>+var iterator_count = 0;
interCaps please.

>- *   Add functions to call by invoking "onLoadRegistry.append(XXXLoadFunc);"
Nit: [].append is not a function, perhaps you were thinking of .push()?

>   var formTree = document.getElementById("formtree");
>@@ -480,15 +498,30 @@
>   linkTree.treeBoxObject.view = linkView;
>   imageTree.treeBoxObject.view = imageView;
Nit: makeTabs is the wrong place for this one-time initialization. Plus you can simplify this to theTree.view = theView;

>-  var iterator = aDocument.createTreeWalker(aDocument, NodeFilter.SHOW_ELEMENT, grabAll, true);
>+  //var iterator = aDocument.createTreeWalker(aDocument, NodeFilter.SHOW_ELEMENT, grabAll, true);
>+  var iterator = newIterator(aDocument);
Why not simply put iteratorCount++ here?

>   setTimeout(doGrab, 16, iterator);
So, all frames are processed in parallel? But I guess that's beyond the scope of this bug.

> function doGrab(iterator)
> {
>   for (var i = 0; i < 50; ++i)
>     if (!iterator.nextNode())
>-      return;
>+      return iterator_count--;
Rather than that timeout hack, I'd prefer {
  if (!--iteratorCount)
    finished();
  return;
}
In particular your code doesn't always return a value (JS strict warning).
Attachment #208790 - Flags: superreview?(neil) → superreview-
Attachment #208790 - Flags: review?(mconnor)
Assignee: db48x → f.qu
This has been fixed as part of bug 339102.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: