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

RESOLVED FIXED

Status

()

Firefox
Page Info Window
--
minor
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: db48x, Assigned: florian)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

13 years ago
Created attachment 207770 [details] [diff] [review]
322610-1.diff
Attachment #207770 - Flags: review?
(Reporter)

Updated

13 years ago
Attachment #207770 - Flags: review? → review?(mconnor)

Updated

13 years ago
Attachment #207770 - Flags: review?(mconnor) → review+
(Reporter)

Comment 2

13 years ago
Created attachment 208790 [details] [diff] [review]
322610-2.diff

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?
(Reporter)

Updated

13 years ago
Attachment #208790 - Flags: review? → review?(mconnor)

Comment 3

13 years ago
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-

Updated

13 years ago
Attachment #208790 - Flags: review?(mconnor)
(Assignee)

Updated

11 years ago
Assignee: db48x → f.qu
(Assignee)

Comment 4

11 years ago
This has been fixed as part of bug 339102.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.