Closed Bug 285171 Opened 20 years ago Closed 20 years ago

Merge MIME type getting code in nsObjectFrame

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: Biesinger, Assigned: Biesinger)

Details

(Keywords: memory-footprint)

Attachments

(1 file, 1 obsolete file)

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.
Attached patch patch (obsolete) — Splinter Review
Can I simply return rv if GetMIMEType fails? or will that cause problems, since
nsObjectFrameSuper::Init ran already?
Attachment #176600 - Flags: superreview?(bzbarsky)
Attachment #176600 - Flags: review?(bzbarsky)
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)
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta2
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?
(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 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.
Attachment #176600 - Flags: superreview?(bzbarsky)
Attachment #176600 - Flags: superreview+
Attachment #176600 - Flags: review?(bzbarsky)
Attachment #176600 - Flags: review+
Priority: -- → P2
Attached patch final patchSplinter Review
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.
Attachment #176600 - Attachment is obsolete: true
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
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: