194281 - [AxPlugin] Issues with Shockwave Flash control

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

A couple of issues with the Shockwave Flash Control have surfaced:

1. The plugin doesn't think it is safe for scripting even though it is.
2. The player is trying to obtain IHTMLWindow2 and call get_Location on it.

The first indicates a problem with the safe for scripting checks when confronted
with an 'old style' control which uses the registry to mark itself safe.

The second will require implementation of get_Location and the IHTMLLocation
object that it returns.

Comment 1

[Charles Rosendahl]

AxPlugin -> Ashish

Comment 2

[Adam Lock]

Attachment: Trivial patch

1-character patch to fix the first issue. I wasn't copying the clsid into the
proper variable.

Comment 3

[Adam Lock]

Comment on attachment 115150 [details] [diff] [review]
Trivial patch

David & Alec, can I have an r/sr for this 1 character patch please? Thanks

Comment 4

[Alec Flett]

Comment on attachment 115150 [details] [diff] [review]
Trivial patch

sr=alecf

Comment 5

[Adam Lock]

Attachment: Patch2

This is the patch for point 2. It's larger, being an implementation of the
IHTMLLocation interface and cleanup of a few methods on IHTMLWindow2 &
IHTMLDocument2.

The implementation of IHTMLLocation has been stuck in a template class. This
seems to be the way to go from now on since I can share code between the DOM
implementations in the control and the plugin.

Comment 6

[David Bradley]

Comment on attachment 115150 [details] [diff] [review]
Trivial patch

r=dbradley

Comment 7

[Adam Lock]

Comment on attachment 115150 [details] [diff] [review]
Trivial patch

Requesting 1.3 approval. Fix is ActiveX specific, no risk and obvious.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 115150 [details] [diff] [review]
Trivial patch

a=asa (on behalf of drivers) for checkin to 1.3

Comment 9

[Adam Lock]

First patch checked into trunk and 1.3 branch

Comment 10

[Steven Bush]

I don't know if this should be filed as a new bug or not, but on the ESPN.com,
they recently instituted a Flash based poll question.  The poll question appears
below the limit of the first page, so the main window has to be scrolled
vertically in order to view it (at least at my current resolution of 1280x1024).
 When I attempt to click on any of the buttons within the flash box, it misses.
 In other words, I have to left mouse click about 2 inches away (further down)
from the spot where it appears I should be clicking in order to work.

Is it possible the GetLocation is not taking into account the current scroll
state of the windows?

Let me know if this should be filed as a separate bug.

Example site:  www.espn.com which uses the Flash 6 plug-in.  If the SportsNation
poll on the right hand side is hidden, scroll the main window down and attempt
to select any of the entries.
OS: Win98
current BuildID: 2003030604 
about Plug-in report for Shockwave:
File name: NPSWF32.dll
Shockwave Flash 6.0 r65

Comment 11

[Adam Lock]

If the ESPN.com poll thing is something that affects existing Netscape / Mozilla
browsers it sounds specific to the NPAPI or the plugin. This bug is specifically
concerned with the flash player misbehaving when my plugin hosts the control
inside Gecko.

Comment 12

[Adam Lock]

Comment on attachment 115158 [details] [diff] [review]
Patch2

Hi David & Alec, can you r / sr this patch to implement the IHTMLLocation
object in the DOM exposed by the plugin? This object is requested by the
Shockwave control during some link click operations.

Thanks

BTW I've fixed the newline problem at the end of the header file.

Comment 13

[Alec Flett]

Comment on attachment 115158 [details] [diff] [review]
Patch2

+template<class T>
+class IHTMLLocationImpl :
+    public IDispatchImpl<IHTMLLocation, &__uuidof(IHTMLLocation),
&LIBID_MSHTML>

I don't see where <T> is used
sr=alecf with it either removed or documented (I'm no activeX expert either, so
I could see if this was required for some reason)

Comment 14

[Adam Lock]

The T is just there for consistency with other I*Impl templates and might be
used someday to call out to methods in the derived class.

David, can you review this too please? Thanks

Comment 15

[David Bradley]

Comment on attachment 115158 [details] [diff] [review]
Patch2

r=dbradley

Is it worth putting in an assert/precondition for pData in Init that it's not
null?

Comment 16

[Adam Lock]

Added assertion and checked in. Thanks all, marking fixed