Comment on attachment 279129
preliminary, incomplete implementation

>Index: layout/style/nsCSSStyleSheet.cpp
>+    aContent.Append(NS_LITERAL_STRING("/* Rules from linked stylesheet, original URL: */\n"));

As I said, this is no good if the linked stylesheets have @namespace rules (or @charset or @import, but skipping those won't necessarily break things, while skipping @namespace most definitely will).  Please make sure to write tests for this case.

What you probably want to do instead is to serialize the rules, and have @import serialization start the serialization of the imported sheet.  I believe that's what dbaron suggested too.

>+  for (i = 0; i < styleRules; ++i) {
>+    rv = GetStyleRuleAt(i, *getter_AddRefs(rule));

Declare |rule| here, not up at the beginning somewhere?

>Index: layout/style/nsHTMLCSSStyleSheet.cpp
>+  NS_IMETHOD Serialize(nsAString& aContent, nsIDocumentEncoderFixup *aFixup);

Why not just put this method on nsICSSStyleSheet, since those are all you serialize?

>Index: layout/style/nsICSSRule.h

And probably put this on nsICSSStyleRule.

I don't see any rule implementations of Serialize() in this patch.  Some of the rules will require fixup too, of course (@document rules come to mind).

Of course such rules in user sheets won't work right once saving has happened, but getting that to work is food for another bug, I think.

>Index: embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
>+#include "nsIFormControl.h"
>+#include "nsIDOM3Node.h"

This looks like part of another patch, right?

>+        // Leave mCurrentDataPath alone so that CSS fixup knows where to save
>+        //mCurrentDataPath = oldDataPath;

This is pretty questionable.  I'd have to dig to make sure, but I would be it's wrong.

>@@ -2504,17 +2522,37 @@ nsWebBrowserPersist::EnumPersistURIs(nsH
>+        rv = cos->Init(outputStream, nsnull, 0, 0);
>+        NS_ENSURE_SUCCESS(rv, rv);

This is wrong if the sheet has an @charset rule. Unless you plan to strip those, in which case it's wrong if the main document is not UTF-8.  Again, please add tests.

>+        PRBool wroteFullString = PR_FALSE;
>+        rv = cos->WriteString(data->mContents, &wroteFullString);

Why is wroteFullString being ignored?  Perhaps this should be called something else?

>+        rv = cos->Flush();
>+        rv = outputStream->Flush();
>+        rv = cos->Close();

Why bother assigning to rv if you plan to ignore it?

>+                    nodeAsLink->GetType(type);
>+                    if (type.EqualsLiteral("text/css")) {

This is wrong.  |type| could be an empty string for a CSS style sheet.  Further, if type is "text/css" there ould be no DOMStyleSheet attached to the node.  You probably want to just GetSheet() and then null-check it.  Again, add tests.

>+                        nsAutoString content;
>+                        ss->Serialize(content, mFixup);
...
>+                        data->mContents.Assign(content);

Why not get the |data| first and just pass data->mContents to Serialize()?

I haven't reviewed the URI-munging details.

CSS fixup should probably also be applied to "style" attributes.  Might be a separate bug.

To answer your quesions in the bug:

> I've tried to avoid gratuitous refactoring of nsWebBrowserPersist

If doing said refactoring (in a separate patch prior to implementing this) would make things better, please go for it!  Unless you think you want to get this into 1.9 and that would make it harder; in that case please file a followup on the refactoring.

> for things like background-image: url(blah), I'm not sure whether CSS knows
> what the mime type of blah is.

For images in particular, it does if they were used and we've gotten far enough in downloading them to know the type.  More precisely, given an nsCSSValue::Image you can get its imgIRequest and get the type from that.

But really, if we're persisting those images (which we should be, right?), we'll know the type because those we _do_ get via an nsIChannel.