neil@parkwaycc.co.uk Reporter
	Description • 23 years ago

AFAIK this should be possible using a data: URL

	rubydoo123
	Comment 1 • 23 years ago

you don't need to save to preview, you do need to save to browse -- is that the
option you are refering to?

	neil@parkwaycc.co.uk Reporter
	Comment 2 • 23 years ago

Yes, sorry, I was thinking of 4.x

	Kathleen :Brade
	Comment 3 • 23 years ago

In 4.x, you needed a saved document to browse.  This is true in 6.x also.

The question we need to investigate for this bug is whether we can use a data url 
instead of forcing the user to save.  This isn't a high priority bug right now 
but we would welcome a patch.

	rubydoo123
	Comment 4 • 23 years ago

future

	Charles Manske
	Comment 6 • 23 years ago

Very interesting trick! First, we would want this as a separate method so it
can be shared by Print and Send Page commands, which also have the same problem
of requiring saving before the command.
Note that these lines in each of those commands:
 if (!CheckAndSaveDocument(window.editorShell.GetString("BeforePreview"),
DocumentHasBeenSaved()))
    return;
should be removed if we use this, since that is what prompts for the user to 
save and the purpose of the fix is to not require that. 
I wonder if there's a problem with the number of characters in the output: Do 
you know if there's a limit on the length of a URL? I noticed some problems in
the Browser when testing this: Since the entire contents of the page is being
placed in the URL string, it tries to display that URL in the Browser's 
location textfield. It seems to have problems with a doc that was 63k long.
Maybe we should modify the Browser's code to test for "data:" urls and not show
the contents in the location field?
Thanks for the help on this, Neil!

	Charles Manske
	Comment 7 • 23 years ago

Modifying Summary to cover all cases where we want to avoid saving the page.

	neil@parkwaycc.co.uk Reporter
	Comment 8 • 23 years ago

The purpose of my fix was to show the current version of the document even if it
hadn't been saved, rather than showing the last saved version. I wasn't sure
whether it was a good idea to remove the prompt altogether so I left it in.

	Charles Manske
	Comment 9 • 23 years ago

Right, the goal is to show the *current* version in Browser, which is why
we don't want to bother the user to save the file!

	neil@parkwaycc.co.uk Reporter
	Comment 10 • 23 years ago

My fault for not asking MPT first :-)

I see no problems with print but I seem to remember that Send Page also sends
the URL of the sent page. If I am right then it wouldn't like a data: URL...

	neil@parkwaycc.co.uk Reporter
	Comment 12 • 23 years ago

Any chance you can check in this bug just for preview and send (why did I say
save?) and then open a new bug for print?

	Charles Manske
	Comment 13 • 23 years ago

I'm still worried about the problems of putting the entire document in the
url, as I noted above, e.g., the bad effects of a long doc in Browser's 
location field. Did you investigate that at all?

	Charles Manske
	Comment 15 • 23 years ago

Latest fix for previewing in browser is better.
Testing "Send Page" causes various asserts in the network code, and sometimes, 
but not always, sending fails.
Unfortunately this still needs more testing, especially to investigate the "Send 
Page" problem.

	Syd Logan
	Comment 16 • 23 years ago

spam composer change

	Charles Manske
	Comment 17 • 23 years ago

Tested code again today: 
1. Load the www.mozilla.org page in browser,
2. Use File | Edit Page  to load into Composer
3. Use File | Send Page  to email it.

It's failing to send the message in nsMsgComposeAndSend::HackAttachments at the 
line:
int status = m_attachments[i].SnarfAttachment(mCompFields);
status is non-zero; Here's the full stack:
nsDebug::WarnIfFalse(const char * 0x0488b908, const char * 0x0488b8b4, const
char * 0x0488b880, int 368) line 396 + 21 bytes
nsURLFetcher::FireURLRequest(nsURLFetcher * const 0x0469be40, nsIURI *
0x0469dc50, nsILocalFile * 0x0469eb90, nsIFileOutputStream * 0x0469dba0,
unsigned int (unsigned int, const char *, const char *, int, const unsigned
short *, void *)* 0x048614c0 FetcherURLDoneCallback(unsigned int, const char *,
const char *, int, const unsigned short *, void *), void * 0x0469d3c4) line 368 + 11
nsMsgAttachmentHandler::SnarfAttachment(nsMsgCompFields * 0x0469c250) line 922 +
77 bytes
nsMsgComposeAndSend::HackAttachments(const nsMsgAttachmentData * 0x00000000,
const nsMsgAttachedFile * 0x00000000) line 2487 + 37 bytes
nsMsgComposeAndSend::Init(nsIMsgIdentity * 0x04c4bd40, nsMsgCompFields *
0x04ce3b10, nsFileSpec * 0x00000000, int 0, int 0, int 0, nsIMsgDBHdr *
0x00000000, const char * 0x04884f7c, const char * 0x00000000, unsigned int 0,
const nsMsgAttachmentData * 0x00000000, const nsMsgAttachedFile * 0x00000000,
const char * 0x100cbe38 gCommonEmptyBuffer) line 2840 + 16 bytes
nsMsgComposeAndSend::CreateAndSendMessage(nsMsgComposeAndSend * const
0x0469f920, nsIEditorShell * 0x04ba75e0, nsIMsgIdentity * 0x04c4bd40,
nsIMsgCompFields * 0x04ce3b10, int 0, int 0, int 0, nsIMsgDBHdr * 0x00000000,
const char * 0x04884f7c, const char * 0x00000000, unsigned int 0, const
nsMsgAttachmentData * 0x00000000, const nsMsgAttachedFile * 0x00000000, void *
...) line 3592 
nsMsgCompose::_SendMsg(int 0, nsIMsgIdentity * 0x04c4bd40, int 0) line 831 + 231
bytes
nsMsgCompose::SendMsg(nsMsgCompose * const 0x04ce7cb0, int 0, nsIMsgIdentity *
0x04c4bd40, nsIMsgProgress * 0x0580e390) line 937 + 20 bytes
XPTC_InvokeByIndex(nsISupports * 0x04ce7cb0, unsigned int 7, unsigned int 3,
nsXPTCVariant * 0x0012bd4c) line 154
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 1952 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x046caba0, JSObject * 0x03332240, unsigned int 3,
long * 0x03610210, long * 0x0012bf84) line 1254 + 14 bytes

	Charles Manske
	Comment 19 • 23 years ago

changing milestone

	Charles Manske
	Comment 20 • 23 years ago

I suspect that the problems with sending mail messages using data: URLs may be
related to bug 102779.

	Charles Manske
	Comment 21 • 23 years ago

load balancing

	Charles Manske
	Comment 22 • 23 years ago

If we still have problems using the "data:" strategy for "Send Page", then I 
suggest we use it only for Preview In Browser. It seems that we can print without
forcing a save, so that's not a problem

	Syd Logan
	Comment 23 • 23 years ago

keeping EDITORBASE

	Matthew T (active 1999-2002)
	Comment 24 • 23 years ago

> I wasn't sure whether it was a good idea to remove the prompt altogether so I
> left it in.

I see the prompt is still in the latest patch -- please do remove it. See bug 
62181 comment 3 onwards for why. Marking dependency.

	Charles Manske
	Comment 25 • 23 years ago

Don't worry! I will remove the prompt in version I checkin.

	Charles Manske
	Comment 27 • 23 years ago

Comment on attachment 61151 [details] [diff] [review]
Updated patch: Just the portion for "Preview Page", updated for bitrot

r=cmanske
(Fix is by Neil)

	Charles Manske
	Comment 28 • 23 years ago

So the plan with the attachment (from #26) is to do just the "Preview in Browser"
part. We don't need to do anything for Print (normal printing uses DOM and thus 
doesn't force us to save). 
We will hold off using "data:" URL to mail the page.

	kinmoz
	Comment 29 • 23 years ago

Comment on attachment 61151 [details] [diff] [review]
Updated patch: Just the portion for "Preview Page", updated for bitrot

Looks ok to me, just a couple of questions:

Why do you have to spell out the following:
"chrome,scrollbars,status,menu,toolbar,directories,sidebar" in the call to
OpenDialog? The old code just did a "chrome,all".

Do we need to wrap the GetContentAs() call with a try/catch just in case the
editor throws an error?

sr=kin@netscape.com

	kinmoz
	Comment 30 • 23 years ago

I should've asked this earlier, but is there any negative impact this can have 
on session history, in the browser window that it brings up? The document we are 
previewing has the potential to be quite large.

	neil@parkwaycc.co.uk Reporter
	Comment 31 • 23 years ago

I listed out the chrome I wanted because I didn't want the location bar, since
the location passed to the browser is a data: URL and not a real location.

	neil@parkwaycc.co.uk Reporter
	Comment 32 • 23 years ago

This seems to work:
window.open(previewURI, 'EditorPreview', 'all,location=no,resizable=yes')
Although I noticed that big data: URLs can get sent as Referrer: headers :-(

	Charles Manske
	Comment 33 • 23 years ago

Comment on attachment 61151 [details] [diff] [review]
Updated patch: Just the portion for "Preview Page", updated for bitrot

I think the problem with referrer means we shouldn't use "data:" URLs to solve
this bug. Milestone changed to investigate using temp file instead.

	Charles Manske
	Comment 34 • 23 years ago

moving milestone

	Kathleen :Brade
	Comment 35 • 23 years ago

removing EDITORBASE per meeting

	Charles Manske
	Comment 36 • 23 years ago

I've inquired with Neil if he can get this done using temp files.

	Charles Manske
	Comment 37 • 23 years ago

changing milestone

	Charles Manske
	Comment 38 • 23 years ago

Changing summary: I think "Browse page" should force saving the file first.

	neil@parkwaycc.co.uk Reporter
	Comment 39 • 23 years ago

This always previews the saved version of the document.
Then, it promptly overwrites the document with the currently edited version!

	Kathleen :Brade
	Comment 40 • 23 years ago

Neil:  be sure to add a comment about the flags you are using (I know you can't
grab the actual flags but put them in a comment so people can easily know what
they are)

	neil@parkwaycc.co.uk Reporter
	Comment 41 • 23 years ago

I always wondered about these constants. Other portions of the code manage to
expose constants to JS, although I don't know how it works, it just does :-)

	neil@parkwaycc.co.uk Reporter
	Comment 42 • 23 years ago

dbradley says "you can declare a list of int constants in the IDL", any help?

	Charles Manske
	Comment 43 • 23 years ago

Neil: it's normally very easy -- you just define some "const ..." in the 
IDL, inside the interface. E.g.:
interface nsIWebProgressListener : nsISupports
{
   const unsigned long STATE_START = 0x00000001;

But the reason you cant do it for GetContentAs is because the flags 
are defined in nsIDocumentEncoder.h; there's no IDL file :(
I suppose we could add them to an editor IDL interface. Remember, btw, that 
nsIEditorShell is being remove, so we shouldn't add new things there.

	Charles Manske
	Comment 44 • 23 years ago

Very interesting idea! I tried it and the browser loaded the modified document,
but then reverted to the content from the file!
The patch is somewhat funky! Only the last 'ComposerCommands.js' is supposed
to be used, right?

	Charles Manske
	Comment 45 • 23 years ago

Not going to happen for 1.0

	Syd Logan
	Comment 46 • 22 years ago

nsbeta1- per buffy triage

	benc
	Comment 47 • 22 years ago

You shouldn't use data: URLs for this.

From RFC2397  The "data" URL scheme:

2. Description

   Some applications that use URLs also have a need to embed (small)
   media type data directly inline. This document defines a new URL
   scheme that would work like 'immediate addressing'.

...

   The "data:" URL scheme is only useful for short values. Note that
   some applications that use URLs may impose a length limit; for
   example, URLs embedded within <A> anchors in HTML have a length limit
   determined by the SGML declaration for HTML [RFC1866]. The LITLEN
   (1024) limits the number of characters which can appear in a single

   attribute value literal, the ATTSPLEN (2100) limits the sum of all
   lengths of all attribute value specifications which appear in a tag,
   and the TAGLEN (2100) limits the overall length of a tag.

6. Security

...
   The effect of using long "data" URLs in applications is currently
   unknown; some software packages may exhibit unreasonable behavior
   when confronted with data that exceeds its allocated buffer size.

	Wayne Mery (:wsmwk)
	Comment 48 • 19 years ago

[mod summary - print no longer requires saving page]

is avoiding "save" before send really useful?
If you're in the process of working on something don't you want it saved to disk to avoid potential lose of your work?

	Matthew T (active 1999-2002)
	Comment 49 • 19 years ago

Sure, replacing the "Save" menu item with automatic saving would be great. But if you're going to keep saving as a manual process, don't force me to save before seemingly-unrelated operations.

	Wayne Mery (:wsmwk)
	Comment 50 • 19 years ago

shouldn't bug 62181 be a blocker or dup rather than dependent?