Closed Bug 285045 Opened 20 years ago Closed 20 years ago

Set the content type on the channel when sniffing it

Categories

(Core Graveyard :: File Handling, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: Biesinger, Assigned: Biesinger)

References

()

Details

Attachments

(1 file, 1 obsolete file)

after we sniffed the type, we should set it on the channel, so that nsITransfer
implementations and the like only see the new type, not the internal
application/x-vnd.mozilla.guess-from-ext type.
Attached patch patch (obsolete) — Splinter Review
Attachment #176509 - Flags: superreview?(darin)
Attachment #176509 - Flags: review?(bzbarsky)
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 176509 [details] [diff] [review]
patch

>Index: nsExternalHelperAppService.cpp
...
>     if (fileExtension.IsEmpty() || mimeType.IsEmpty()) {
>       // Extension lookup gave us no useful match
>       GetFromTypeAndExtension(NS_LITERAL_CSTRING(APPLICATION_OCTET_STREAM), fileExtension,
>                               getter_AddRefs(mimeInfo));
>+      if (channel)
>+        channel->SetContentType(NS_LITERAL_CSTRING(APPLICATION_OCTET_STREAM));
>+    } else if (channel) {
>+      channel->SetContentType(mimeType);
>     }
>   } 

This code isn't perf critical, so how about writing it like this:

    if (fileExtension.IsEmpty() || mimeType.IsEmpty()) {
      // Extension lookup gave us no useful match
      mimeType.AssignLiteral(APPLICATION_OCTET_STREAM);
      GetFromTypeAndExtension(mimeType, fileExtension,
getter_AddRefs(mimeInfo));
    } 
    if (channel)
      channel->SetContentType(mimeType);

It seems more readable to me, and it probably results in less code.

sr=darin either way
Attachment #176509 - Flags: superreview?(darin) → superreview+
Comment on attachment 176509 [details] [diff] [review]
patch

r=bzbarsky
Attachment #176509 - Flags: review?(bzbarsky) → review+
Attached patch patch v2Splinter Review
thanks for the suggestion, I like this way better too.
Attachment #176509 - Attachment is obsolete: true
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <-- 
nsExternalHelperAppService.cpp
new revision: 1.284; previous revision: 1.283
done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: