Webbrowserpersist doesn't like mailto: links

RESOLVED FIXED in mozilla0.9.8

Status

Core Graveyard
File Handling
RESOLVED FIXED
17 years ago
2 years ago

People

(Reporter: Dimitrios, Assigned: Adam Lock)

Tracking

Trunk
mozilla0.9.8
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
Build 2001121303, Win95. Url taken from bug Bug 115069. When saved as "web page
complete" not all frames are saved. No images at all and wrong frameset page.
Other framed pages are saved correctly, I cannot understand it fails on this
one. Please update summary as necessary.
(Reporter)

Comment 1

17 years ago
To ben@netscape.com
Assignee: law → ben
(Assignee)

Comment 2

17 years ago
Problems in mfcEmbed too. I'll take this one.

First impressions suggest the "/" at the front of some of the frameset URLs is
confusing something in the persist code.
Assignee: ben → adamlock
i'm guessing this is prolly cross-platform. but, alas, i cannot really test Save
Frame As on linux due to bug 115154, or on mac due to bug 107521. :(
OS: Windows 95 → All
Hardware: PC → All
(Assignee)

Comment 4

17 years ago
I believe the bug is being triggered because nsWebBrowserPersist::Cleanup() is
being called before the DOM for the main frame and subframes has finished
traversal. This means the list of documents is wiped before they can be saved.
I'm working on a patch which defers all URI persistence until traversal has
completed so Cleanup() is not called at the wrong place.
(Assignee)

Comment 5

17 years ago
Ugh, the problem is being caused because the frames contain this line in the header:

  <link REV="made" HREF="mailto:faq-admin@faqs.org">

Webbrowserpersist is failing when it can't open the mailto: channel. On MFCEmbed
this even causes it to crash because the last ref to the persist object is
released during the call out to the client nsIWebProgressListener::onStateChange.

I'm going to do what I mentioned earlier, plus trap and ignore channels that do
not do content and also put a kung-fu death grip on saveDocument to prevent
premature destruction.
(Assignee)

Comment 6

17 years ago
Created attachment 62033 [details] [diff] [review]
Patch

This patch:

* Puts a kung-fu death grip on the object during SaveDocument
* Throws away mailto: and about: links* Adds explicit check for channels that
have no content

Reviews please
(Assignee)

Comment 7

17 years ago
Ben can you review this patch? Thanks
Keywords: nsbeta1, patch, review
(Assignee)

Comment 8

17 years ago
*** Bug 116757 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

17 years ago
Summary: This framed page cannot be saved correctly → Webbrowserpersist doesn't like mailto: links
(Assignee)

Updated

17 years ago
Depends on: 110135

Comment 9

17 years ago
There are probably other protocols that should be ignored:
  javascript:
  data:

Here is a list of protocols that I've generated in the past which should be
considered for this patch:
  file, http, https, ftp,
  javascript, about, mailto, news, snews,
  ldap, ldaps, gopher, finger, telnet
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla0.9.8

Comment 11

17 years ago
and mailbox:
(Assignee)

Comment 12

17 years ago
Created attachment 65499 [details] [diff] [review]
Work in progress

I have attached a patch that demonstrates how I intend to fix this bug, but a
dependency on bug 119496 means it won't apply cleanly so don't bother trying.
Consider it work in progress.

Basically the patch creates an C++ persist policy class that webbrowserpersist
calls to ask whether a particular URI should be saved or not. The class tests
for most of the schemes that have been listed earlier here in this bug. I
envisage this class expanding into a full-blown object with lots of methods for
bug 113902 for this bug it's just a simple class.
(Assignee)

Updated

17 years ago
Keywords: review

Comment 13

17 years ago
preliminary r=brade (assuming you add mailbox:)
(Assignee)

Comment 14

17 years ago
*** Bug 121585 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 15

17 years ago
The checkin I made for bug 115216 last night contained a 'lite' version of this
patch which checks all the schemes mentioned above plus "irc:" but inplace
instead of in a seperate C++ policy class. That is really an issue work for bug
113902 to sort out.

Marking fixed.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
over to benc to verify for the other protocol cases.

my other bugs dependent on this appear fixed.
QA Contact: sairuh → benc
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.