The nsDOMWalker class in webbrowserpersist should be turned into an XPCOM component. It's too useful to deny it to other code!
Created attachment 89711 [details] [diff] [review] Patch Patch creates a new nsIDOMWalker & nsIDOMWalkerCallback interface and fixes up the class to be an XPCOM object
Created attachment 89933 [details] [diff] [review] Updated patch Updated patch verified on Mac, added some documentation to nsIDOMWalker interface
Attachment #89711 - Attachment is obsolete: true
Comment on attachment 89933 [details] [diff] [review] Updated patch Adam says he has the changes to the Mac IDL projects and, as long as they build, r=ccarlen.
Attachment #89933 - Flags: review+
Comment on attachment 89933 [details] [diff] [review] Updated patch firstname.lastname@example.org --- Should aAbort be an |out| parameter instead of |inout|? Or was that on purpose? + void onWalkDOMNode(in nsIDOMNode aNode, inout boolean aAbort);
Attachment #89933 - Flags: superreview+
Yes, inout is deliberate. If it were just out the implementor would have to explicitly set this value each time during the call. Making it inout only means they have to set it if they want to cancel.
Fix is checked in. Verification is to ensure that saving a web page with all images still works.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Note that naming interfaces with nsIDOM* when they're not actually DOM interfaces is a bad idea. We'll end up treating them as a DOM interface in nsDOMClassInfo while they're not.
Yes, Jonas is right here, interfaces named nsIDOM* are special in Mozilla, *don't* pollute that namespace with non-*DOM* stuff. Reopening, this interface needs to be renamed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
And from what I can tell from this interface, Jonas is right in that you could probably use the existing standard tree walker code here, no need to re-invent the wheel.
Apologies, I'll work up another patch. I didn't know we had an official tree walker, nsDOMWalker has been around for a long while and probably predated it.
Created attachment 90071 [details] [diff] [review] Patch to remove DOM walker This patch removes nsIDOMWalker and nsDOMWalker entirely. I've changed the webbrowserpersist object to use the nsIDOMTreeWalker. Can I have r/sr on this please?
Comment on attachment 90071 [details] [diff] [review] Patch to remove DOM walker the patch is missing the actual removal of the idl and cpp/h files. But with that r=sicking
Attachment #90071 - Flags: review+
Comment on attachment 90071 [details] [diff] [review] Patch to remove DOM walker Awesome! Thank you Adam for jumping on this and fixing this so quickly! sr=jst
Attachment #90071 - Flags: superreview+
Fix checked in. nsDOMWalker & interface is history.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago → 16 years ago
Resolution: --- → FIXED
To ashish for bug verification
QA Contact: mdunn → ashishbhatt
You need to log in before you can comment on or make changes to this bug.