285171 - Merge MIME type getting code in nsObjectFrame

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Currently, various places in nsObjectFrame get the mime type for the content (in
different ways, even!). The attached patch merges them by creating a new
GetMIMEType function.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Can I simply return rv if GetMIMEType fails? or will that cause problems, since
nsObjectFrameSuper::Init ran already?

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Also, can GetOwnerDoc ever return null?


I went with the method that created a URI and passed it to GetTypeFromURI, btw
(instead of the one that hand-parsed the extension out and used
GetTypeFromExtension)

Comment 3

[Boris Zbarsky [:bzbarsky]]

GetOwnerDoc() won't return null on the mContent of a frame.  At least it better
not.  ;)

As for Init()...  We shouldn't be throwing from Init() just because we have no
type, I don't think.  It's perfectly valid to have an <embed> with no type attr
and no obvious type in the URI, no?

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #3)
> It's perfectly valid to have an <embed> with no type attr
> and no obvious type in the URI, no?

Apart from the fact that we do nothing useful with it (unless, maybe, it's
handled by a plugin)? Sure :)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176600 [details] [diff] [review]
patch

>@@ -674,16 +623,21 @@ nsObjectFrame::Init(nsPresContext*   aPr
>+  nsCAutoString type;
>+  rv = GetMIMEType(type);
>+  // XXX what if that fails?

Then we shouldn't run any of the code that looks at |type|, imo.  So wrap it in
|if (NS_SUCCEEDED(rv))| ?

>+  nsresult GetMIMEType(nsACString& aType);

NS_HIDDEN_(nsresult), maybe?

r+sr=bzbarsky with those nits.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: final patch

I went with:
+++ generic/nsObjectFrame.cpp	10 Mar 2005 14:53:34 -0000
@@ -630,7 +630,9 @@
   // Find our content type
   nsCAutoString type;
   rv = GetMIMEType(type);
-  // XXX what if that fails?
+  // If that fails, just return and render nothing.
+  if (NS_FAILED(rv))
+    return NS_OK;

   // Ideally this should move to Reflow when the stream starts to come
   if (IsSupportedImage(type)) {

And I also removed the XXX comment about ownerDocument.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in generic/nsObjectFrame.cpp;
/cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v  <--  nsObjectFrame.cpp
new revision: 1.491; previous revision: 1.490
done
Checking in generic/nsObjectFrame.h;
/cvsroot/mozilla/layout/generic/nsObjectFrame.h,v  <--  nsObjectFrame.h
new revision: 1.37; previous revision: 1.36
done