309529 - Move logic from nsObjectFrame::Instantiate(const char*, nsIURI*) into nsObjectLoadingContent

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

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

nsObjectFrame::InstantiatePlugin doesn't have much code anymore; most of it can
probably be moved to nsObjectLoadingContent. it would allow to remove some
redundancy (multiple conversions from classid to MIME type, etc)

Comment 1

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

this probably includes figuring out what this odd base URI business in that
function is doing (the base URI is passed to pluginhost->Instantiate for the
case where a class ID was present and supported)

Comment 2

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

ok, as best as I can tell the activex code does not care at all what data it gets:
http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/plugin/LegacyPlugin.cpp#1131

Comment 3

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

hm, the plugin host really does want a non-null URI...

Comment 4

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

Attachment: testcase: java with codebase attribute

Comment 5

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

Attachment: patch

Comment 6

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

oh, I switched back from nsIURL for the extension to the look-for-last-dot code, because I was thinking that some real cases here might not implement nsIURL.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

>Index: layout/generic/nsObjectFrame.cpp
>@@ -1771,67 +1771,15 @@ nsObjectFrame::Instantiate(const char* a

If this method now expects a non-null nsIURI and a nonempty type, please assert those here?  And document on nsIObjectFrame?

>Index: content/base/src/nsObjectLoadingContent.cpp
>+nsObjectLoadingContent::Instantiate(const nsACString& aMIMEType, nsIURI* aURI)
>+    aURI->GetSpec(spec);
>+
>+    PRInt32 offset = spec.RFindChar('.');

http://something?foo=bar.baz

Do we really want to come out with "baz"?

That is, I think we should try for nsIURL and getting the extension.  Only if that fails should we consider hand-parsing this....

r+sr=bzbarsky with those fixed...

Comment 8

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

what it expects is an (internally-handleable) URI or a nonempty type... i.e. only one of them needs to be nonnull. will document.

Comment 9

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

Attachment: patch v2

Comments addressed, plus:
- Removes the now unused MakeAbsoluteURI
- reorders the type initialization (if empty) and null URI handling, so that we don't try to get an extension off the base URI

rerequesting review since this isn't just addressing the requested changes. should be straightforward though :)

Comment 10

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

Attachment: patch v2.1

merged to trunk (after sicking's changes to GetAttr)

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 201199 [details] [diff] [review]
patch v2.1

r+sr=bzbarsky.  I checked that this compiles too.

Comment 12

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

Checking in content/base/public/nsIObjectLoadingContent.idl;
/cvsroot/mozilla/content/base/public/nsIObjectLoadingContent.idl,v  <--  nsIObjectLoadingContent.idl
new revision: 3.2; previous revision: 3.1
done
Checking in content/base/src/nsObjectLoadingContent.cpp;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v  <--  nsObjectLoadingContent.cpp
new revision: 1.10; previous revision: 1.9
done
Checking in content/base/src/nsObjectLoadingContent.h;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.h,v  <--  nsObjectLoadingContent.h
new revision: 1.3; previous revision: 1.2
done
Checking in layout/generic/nsIObjectFrame.h;
/cvsroot/mozilla/layout/generic/nsIObjectFrame.h,v  <--  nsIObjectFrame.h
new revision: 1.8; previous revision: 1.7
done
Checking in layout/generic/nsObjectFrame.cpp;
/cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v  <--  nsObjectFrame.cpp
new revision: 1.527; previous revision: 1.526
done
Checking in layout/generic/nsObjectFrame.h;
/cvsroot/mozilla/layout/generic/nsObjectFrame.h,v  <--  nsObjectFrame.h
new revision: 1.48; previous revision: 1.47
done