Make nsIMIMEInfo::MIMEType readonly

RESOLVED FIXED in mozilla1.8alpha2

Status

Core Graveyard
File Handling
P2
normal
RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: Biesinger, Assigned: Biesinger)

Tracking

Trunk
mozilla1.8alpha2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reasoning: The default application is now immutable. So one instance of a
mimeinfo will always have the same default application, even though the mimetype
can be changed. But this new mime type may actually have a different default
application. I'd like to avoid that situation.
(this requires a few changes to the helper application preference code)
Priority: -- → P2
Target Milestone: --- → mozilla1.8alpha
hm, comment 1 is actually not true, I wonder why...
Because it doesn't use MIME infos.  It's eeeevil.

How do we plan to make it work once it _does_ use MIME infos?

(In reply to comment #3)
> Because it doesn't use MIME infos.  It's eeeevil.

ew, right, forgot about that...

> How do we plan to make it work once it _does_ use MIME infos?

get a new mimeinfo from the mimeservice for the new type.
changing the mime type should be a rare thing, I imagine...
That works, I guess.  As long as we keep the extensions writable...
note to self: exthandler sets the mimetype in these places: (with a small patch)
GetMIMEInfoForExtensionFromDS
GetFromTypeAndExtension:
  DS lookup via extension
  Extras lookup via extension
GetMIMEInfoForExtensionFromExtras
Created attachment 149103 [details] [diff] [review]
something along this lines, maybe (DO NOT CHECK IN)

I'm thinking of something like this.

needs a bit of work - GetTypeFromExtension needs to look at the RDF datasource
as well; and beos and mac won't compile with this patch.
Created attachment 149107 [details] [diff] [review]
like this

ok, this should work
Attachment #149103 - Attachment is obsolete: true
Attachment #149107 - Flags: review?(bzbarsky)
Status: NEW → ASSIGNED
It might take me a week or so to get to this...  I spent a good chunk of
yesterday reviewing.  :(
Target Milestone: mozilla1.8alpha1 → mozilla1.8alpha2
Comment on attachment 149107 [details] [diff] [review]
like this

>Index: uriloader/exthandler/nsExternalHelperAppService.cpp
>+PRBool nsExternalHelperAppService::GetTypeFromDS(const nsACString& 

Wouldn't it be better to return the actual error codes instead of turning them
all into PR_FALSE?

This is fine either way, in any case.

>Index: uriloader/exthandler/mac/nsOSHelperAppService.cpp

>@@ -263,21 +267,22 @@ nsOSHelperAppService::GetMIMEInfoFromOS(
>+    nsMIMEInfoMac* mimeInfoMac = new nsMIMEInfoMac();
>+    if (!mimeInfoMac)
>       return nsnull;
>-    NS_ADDREF(mimeInfo);
>+    NS_ADDREF(mimeInfoMac);
> 
>     if (!aMIMEType.IsEmpty())
>-      mimeInfo->SetMIMEType(aMIMEType);
>+      mimeInfoMac->SetMIMEType(aMIMEType);

Why not just pass aMIMEType to the constructor?

r=bzbarsky with that.  Sorry this took so long...
Attachment #149107 - Flags: review?(bzbarsky) → review+
(In reply to comment #10)
> Wouldn't it be better to return the actual error codes instead of turning them
> all into PR_FALSE?
> 
> This is fine either way, in any case.

hmm, the callers don't really care about the exact error code...

> Why not just pass aMIMEType to the constructor?

ah, yeah, good point.  I really should add a ctor that takes a const nsACString&.
Created attachment 150027 [details] [diff] [review]
patch v2
Attachment #149107 - Attachment is obsolete: true
Attachment #150027 - Flags: superreview?(darin)

Comment 13

14 years ago
Comment on attachment 150027 [details] [diff] [review]
patch v2

>Index: uriloader/exthandler/nsExternalHelperAppService.cpp

>+  NS_ConvertUTF8toUCS2 extension(aExtension);

nit: use NS_ConvertUTF8toUTF16 instead.


>Index: uriloader/exthandler/nsMIMEInfoImpl.cpp

> nsMIMEInfoBase::~nsMIMEInfoBase()
> {
> }

nit: no need to code the dtor right?


sr=darin
Attachment #150027 - Flags: superreview?(darin) → superreview+
(In reply to comment #13)
> >Index: uriloader/exthandler/nsMIMEInfoImpl.cpp
> > nsMIMEInfoBase::~nsMIMEInfoBase()
> > {
> > }
> 
> nit: no need to code the dtor right?

as I want it to be virtual, I do need an impl; I'll put it inline in the .h file.
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.