Re: content type changes

[Doug Turner]

 

Judson Valeski wrote:

I plowed through the diffs between DOUGT_CHANNEL_CHANGES_01222001_BASE and DOUGT_CHANNEL_CHANGES_01222001_BRANCH, looking for semantic changes in ContentType setting and retrieval. These specific changes will impact consumer expectations and are the core of my concern. The diffs I perused don't show new files you may have added; did you add new files to your branch?

Yes, I did add a new file: netwerk/base/public/nsIStreamContentInfo.idl

 
Below are diffs I have questions about based on the context set above. I didn't do a full review of the changes (you must be beat after making 750k worth of diffs :-)).

Index: dom/src/jsurl/nsJSProtocolHandler.cpp
===================================================================
RCS file: /cvsroot/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp,v
retrieving revision 1.55
retrieving revision 1.55.34.1
diff -u -r1.55 -r1.55.34.1
--- nsJSProtocolHandler.cpp 2000/09/13 23:50:39 1.55
+++ nsJSProtocolHandler.cpp 2001/01/23 20:35:59 1.55.34.1
@@ -492,11 +492,6 @@
     nsCOMPtr<nsIStreamIOChannel> channel;
     rv = NS_NewStreamIOChannel(getter_AddRefs(channel), uri, thunk);

-    // If the resultant script evaluation actually does return a value, we
-    // treat it as html.  XXXbe see <plaintext> comment above
-    if (NS_SUCCEEDED(rv)) {
-        rv = channel->SetContentType("text/html");
-    }
     if (NS_SUCCEEDED(rv)) {
         rv = thunk->Init(uri, channel);

     }
You've pulled the default mime type from the JS handler. I don't know the JS handler at all, but I'm concerned that consumers are going to get ahold of a channel/request before the type has been set, and that this will wreak havoc. Do JS loads happen via the uriLoader? If so, are you assuming (probably correctly) that the unknowndecoder will resolve the type to text/html?

As you know, the branch is not finished yet and has not been reviewed.  Setting the content type of a channel that you do not implement is a edge case.  I still need to fix the uriloader to deal with this.

 
Index: extensions/psm-glue/public/psm-glue.js
===================================================================
RCS file: /cvsroot/mozilla/extensions/psm-glue/public/psm-glue.js,v
retrieving revision 1.1
retrieving revision 1.1.60.1
diff -u -r1.1 -r1.1.60.1
--- psm-glue.js 2000/03/28 12:24:17 1.1
+++ psm-glue.js 2001/01/23 18:23:53 1.1.60.1
@@ -9,3 +9,5 @@
 pref("security.warn_leaving_secure",     true);
 pref("security.warn_viewing_mixed",      true);
 pref("security.warn_submit_insecure",    true);
+
+pref("security.ui.enable",    true);

Just a sanity check. There are some PSM changes (above is just one) in this branch, should they be?
 

Yeah, ignore this.  It shouldn't be on my branch.

Index: layout/xml/document/src/nsXMLDocument.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xml/document/src/nsXMLDocument.cpp,v
retrieving revision 1.94
retrieving revision 1.94.2.1
diff -u -r1.94 -r1.94.2.1
...
@@ -375,8 +381,10 @@
   rv = aChannel->GetURI(getter_AddRefs(aUrl));
   if (NS_FAILED(rv)) return rv;

-  rv = aChannel->GetContentType(&aContentType);
-
+  nsCOMPtr<nsIMIMEService> MIMEService (do_GetService(NS_MIMESERVICE_CONTRACTID, &rv));
+  if (NS_FAILED(rv)) return rv;
+  rv = MIMEService->GetTypeFromURI(aUrl, &aContentType);
+
   if (NS_SUCCEEDED(rv)) {
     if ( 0 == PL_strcmp(aContentType, "text/html")) {
    bIsHTML = PR_TRUE;

The MIMEService simply maps a file extension against a table of extension to mime type mappings. So, if the URI is http://www.foo.com/bar , and bar is actually an xml file, before this change, the server would have likely set the content type in the channel to "text/xml." Your change will result in the "unknown/type" mime type to be set in aContentType. If this load goes through the uriloader, and eventually gets to the unknowndecoder, the unknown content type handler dialog will be thrown because the unkonwndecoder doesn't decode xml.

<snip>

 
Same as the previous two.

The order of precedence on content type resolving (everywhere in the system) is this:
Server header (applies to HTTP only as other protocols we support don't provide content type info)
File extension mapping (or Internet Config on the mac)

These are all bad examples, Jud.  In every case, the server has not been opened yet!  :-/  Which means that the mime service is going to be called.  Yes, that is right, the http protocol is doing the same thing today:

http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/src/nsHTTPChannel.cpp#411

It is wrong to have this hidden via the protocol.