200680 - [AxPlugin] Problem using WMP on this site

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

This site has some straightforward JS to embed WMP to play a radio stream:

http://www.redfm.ie/radiostream.html

Works fine in IE, but in Moz I get a black box where the WMP is supposed to be.
Possibly there is something about the parameters or the relative link to the
redfm.asx filename or the asx content itself which means the control is created
but cannot connect.

Content of http://www.redfm.ie/redfm.asx is:

<ASX VERSION = "3.0">
<title>RedFM 104 - 106 Cork's #1 For Hit Music</title>
<entry>
<copyright>(c) RedFM 2002</copyright>
<moreinfo href="http://www.redfm.ie" />
<REF href="http://wmp-live1.redfm.ie/redfm" />
</entry>
<entry>
<title>RedFM 104 - 106 Cork's #1 For Hit Music</title>
<copyright>(c) RedFM 2002 (server 2)</copyright>
<moreinfo href="http://www.redfm.ie" />
<REF href="http://wmp-live1.redfm.ie/redfm" />
</entry>
</ASX>

Comment 1

[Adam Lock]

WMP definitely has a probelm resolving the relative "FileName" param onto the
base address. Even if I replace the relative name with an absolute URL, it
crashes later on.

I'll have to debug further to figure the exact reason.

Comment 2

[Adam Lock]

Actual error reported by WMP with the relative URL.

Error number: -2147220891
Error description: Cannot play back the file.  The format is not supported.

Problem appears to be in XPCDocument.cpp's implementation of
IBindHost::CreateMoniker.

WMP is calling this method to create a URL moniker from "redfm.asx", expecting
the implementation to tack on the "http://www.redfm.ie/" part. Since it doesn't,
the resulting URL moniker points nowhere invalid and causes an error.

Comment 3

[Adam Lock]

Attachment: Patch 1

Issue was two things:

1. IBindHost::CreateMoniker as described previously stopped the asx stream URL
from being found.
2. Uninitialised structure caused WMP to crash further in when it called out to
query some type information from the plugin.

Patch fixes both of these issues. The CreateMoniker class now creates a moniker
from the base URL and uses that when handing them out. The MozActiveX.cpp file
contains calls in DllMain to initialise and cleanup the ATL CComModule
instance.

There still appears to be a problem rendering the player the same way it
appears in IE. Perhaps the mass of params (56 in total!) is not being fed in
properly to WMP, is altering its appearance? I'll check that next since it
could be a plugin layer problem.

Comment 4

[Adam Lock]

The remaining rendering issue appears to be because the WMP ignores the plugin
telling it not to open in windowless mode and doing it anyway.

The simple fix is to not expose the plugin control site's
IOleInPlaceSiteWindowless interface at all to force WMP to use windowed mode. A
better fix is to make the windowless functionality in the plugin work properly.

Removing IOleInPlaceSiteWindowless works fine but it forces the control to
render in the WMP 'classic' style and not utilise any fancy skin rendering. I'll
see if the windowless rendering can be made to work properly.

Comment 5

[Adam Lock]

Attachment: Patch 2

This is the easy way to fix the problem. Patch disables windowless support
entirely by not tempting controls to misbehave by exposing the windowless
interface.

I shall raise another bug to cover fixing the issue the hard way by
implementing windowless mode properly.

Comment 6

[Adam Lock]

Comment on attachment 119690 [details] [diff] [review]
Patch 1

Can I have an r/sr on this patch for the plugin? The patch initialises a global
variable called _Module used by ATL and fixes some code that returns monikers
(a cross between an nsIURI and an nsIRequest in COM) to use the base address
when fixing up URLs.

Thanks

Comment 7

[Adam Lock]

Comment on attachment 119858 [details] [diff] [review]
Patch 2

Can I have an r/sr on this simple patch please? It hides the
IOleInPlaceSiteWindowless interface on the site object so that WMP can't abuse
it by calling windowless methods when it has already been told to open in
windowed mode.

Comment 8

[Alec Flett]

Comment on attachment 119858 [details] [diff] [review]
Patch 2

mind adding a comment to explain why you're leaving the old interface in the
interface, but commented out? (say that 3 times fast)

sr=alecf with the comment

Comment 9

[Alec Flett]

Comment on attachment 119690 [details] [diff] [review]
Patch 1

sr=alecf

Comment 10

[David Bradley]

Comment on attachment 119690 [details] [diff] [review]
Patch 1

r=dbradley

I see a mixture of use of tabs for indention. Init's assertion change looks to
be purely whitespace. If you want to fix them, there's an option in VC++ you
may or may not be aware of, Edit|Advances|Untabify Selection. I didn't find
that until a few months ago.

Comment 11

[David Bradley]

Comment on attachment 119858 [details] [diff] [review]
Patch 2

r=dbradley

I agree with alecf, inject a comment.

Comment 12

[Adam Lock]

Both patches checked in with the suggested extra comment. Bug 201231 deals with
the windowless control issues remaining.