44322 - [WPAPI] Implement Navigator 4.x windowless plug-in API

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, enhancement, P3)

Description

[ekrock's old account (dead)]

Nav4 had a windowless plug-in API that enabled the creation of plug-ins that 
could be transparent, among other things. It was never widely adopted; I've only 
been able to identify seven vendors who ever used it. 

Given the remaining time before RTM, Netscape lacks the staff resources 
necessary to implement this for Netscape 6 FCS. I am opening this bug as a 
[META] tracking bug for this issue and marking it FUTURE. 

Netscape will evaluate the possibility of Netscape engineering resources being 
used to implement this feature for the first post-FCS point release or 
thereafter. In the meantime:

1) There are a currently-undefined number of specific things that need to be 
done in order to get the windowless plug-in API working. If you are aware of a 
specific issue, please open a separate bug report for each issue (e.g. each 
method that must be implemented, etc., so that we can track and prioritize each 
issue independently), make this bug report depend on your bug report, and mark 
your bug report with milestone FUTURE as well.

2) If you are a plug-in vendor who has used the windowless plug-in API and would 
like to see it implemented in Netscape 6 FCS or a subsequent release, please add 
yourself to the cc: list for this bug and email your contact information to 
[e-mail].  

3) If a partner company, a plug-in vendor, and/or members of the mozilla 
community wish to commit to do the work to implement the windowless plug-in API, 
Netscape would welcome their assistance--this is an open source project! 
(Check-in for Netscape 6 FCS would require PDT team "exception feature" approval 
and would require a risk-benefit analysis; demonstrating essentially zero risk 
for the rest of the product would be necessary to gain the approval.) Mozilla 
developer community members should remember that they can get the equivalent 
check-in approval for the Mozilla trunk from [e-mail] or 
[e-mail] (as of 6/30/00).

Comment 1

[ekrock's old account (dead)]

Marking FUTURE and 4xp.

Comment 2

[ekrock's old account (dead)]

Restoring [META]. Hi Braden, the idea for this bug is that it will be a META bug 
for the general issue, and then when we find specific changes A,B,C that must be 
made in order to implement the Windowless Plug-in API, those would be added as 
individual dependency bugs. Added [WPAPI] mark for easier tracking/searching of 
those bugs as they're opened.

Comment 3

[Braden]

ekrock: This bug already has a "meta" keyword attached to it. Why does it need
both that and the summary clutter? I know a lot of CSS summary clutter was
cleaned up with the addition of the CSS keywords; why is "meta" being treated
differently?

Comment 4

[ekrock's old account (dead)]

Oh! I didn't know that we have a meta keyword, and I didn't see it. Thanks! 
Removing [META]. Thanks for the FYI!

Comment 5

[David Brittain]

I have had a go at implementing some of the functions needed for windowless 
plugin support. In particular InvalidateRect and ForceRedraw. Also, the browser 
now reads the transparency variable from the plugin using the GetValue method. 
Using these new methods I have a plugin working on Windows 2000 that is 
windowless, i.e. transparent and not always on top. It's actually a version of 
the swimming fish at 
http://developer.netscape.com/viewsource/goodman_cross/fish_new.htm with a 
plugin instead of a fish.

Comment 6

[David Brittain]

Attachment: Patches for implementation of windowless support

Comment 7

[David Brittain]

Attachment: Zip file containing windowless version of basic plugin (basic.dll), windowless test file (fish.htm) and the modified source

Comment 8

[David Brittain]

I have updated the patch (attached below) with improved error detection, proper 
freeing of interfaces etc. I have also added an implementation for 
nsIPluginInstancePeer::GetValue.

From bug report 62248, I understand that some interfaces have been frozen and 
can't be changed - is this true of nsIPluginInstanceOwner. If so, then as 
suggested there I could add the new functions to a new nsIPluginInstanceOwner2, 
and derive the nsPluginInstanceOwner in nsObjectFrame from that instead.

Also, when is the code in nsPluginViewer used? I have currently left the 
functions in there as not implemented.

Comment 9

[David Brittain]

Attachment: Updated patch for windowless implementation

Comment 10

[David Brittain]

Can someone please review the above patch? Also could a target milestone of
mozilla0.9 be assigned to this bug.

Comment 11

[Braden]

Nominating for mozilla 0.9.

dbrittain: If you want a review, post to netscape.public.mozilla.reviewers.

Comment 12

[David Brittain]

Attachment: Updated patch that fixes a problem caused by another change

Comment 13

[David Brittain]

Attachment: Sorry, attached the wrong file before, please ignore the previous patch this is the correct one.

Comment 14

[David Brittain]

Modified patch fixes a problem introduced during the changes to fix bug 59620. 
It seems that calling SetWindow in nsObjectFrame::DidReflow() causes a problem 
on the Mac, so it was removed. In this patch the call is added back in, but 
only in non-Mac code.

Braden - thanks for the tip about reviewing, I'll try the newsgroup.

Comment 15

[sean echevarria]

*** Bug 70872 has been marked as a duplicate of this bug. ***

Comment 16

[Peter Lubczynski]

[e-mail]:

Looks like you were seeking review for your patches at one time. I'm interested 
in checking this in if it works. Nominating mozilla1.0

Comment 17

[David Brittain]

The patch is probably a bit out of date I haven't tried integrating it into the 
latest build for a while. I am away for a week as of today - I'll update it 
when I get back.

Comment 18

[David Brittain]

Attachment: Updated patch that works with latest build

Comment 19

[David Brittain]

Attachment: Zip file containing modified version of npsimple.cpp for testing windowless plugin

Comment 20

[David Brittain]

Above there is a new patch and new test case. Most of the comments above still 
apply. ie. I have only implemented InvalidateRect, ForceRedraw and not 
InvalidateRegion. I have only tested this on Windows and haven't tried to 
update the test plugin to work on either Unix or Macs, so if anyone can do this 
that would be great.

The patch also includes an implementation of nsIPluginInstancePeer::GetValue ( 
Bug 62248 ). I have tested this with another plugin but it is not used in the 
test code that is attached.

Comment 21

[Peter Lubczynski]

*** Bug 56126 has been marked as a duplicate of this bug. ***

Comment 22

[Arun Ranganathan]

CC'ing [e-mail] because of the nsPluginInstancePeerImpl::GetValue
incidental fix in Brittain's patch.  Can someone verify the patch?

Comment 23

[David Brittain]

Attachment: Updated patch for nsObjectFrame.cpp

Comment 24

[karnaze (gone)]

Moving to m0.9.2.

Comment 25

[Peter Lubczynski]

David,

Chris Karnaze is currently working on nsObjectFrame:Paint(). Your latest patch
may or may not be effected by his new changes. See bug 76085.

Comment 26

[av (gone)]

David, I don't fully understand couple of things, could you please explain?
1. why do we need to do anything in nsObjectFrame::DidReflow?
2. in nsObjectFrame::Paint why that reshuffling of logic? Are you saying that 
there can be a child frame in case of windowless plugin?
3. how is nsPluginInstanceOwner::GetValue supposed to be called?
4. is this patch actually functioning?

Comment 27

[David Brittain]

1. why do we need to do anything in nsObjectFrame::DidReflow?

This is so the correct values are passed into SetWindow. Under NS4.x SetWindow 
expects the nsPluginWindow structure to have x and y set to values that are 
relative to the window containing the windowless plugin. This is because - on 
windows at least - when you paint to the DC you need to know where in the 
window to do it, the x and y values specify the origin of the windowless area 
to paint to.

2. in nsObjectFrame::Paint why that reshuffling of logic? Are you saying that 
there can be a child frame in case of windowless plugin?

I made this change in the last patch I submitted. What I found was that in the 
test plugin, which is attached above, Paint was no longer getting called. It 
seems that there is a child frame in this case, as the method call always 
follows that path - I have to admit I'm not sure where it has come from. The 
previous patches I submitted worked without the need to do this. Anyone got any 
ideas?

3. how is nsPluginInstanceOwner::GetValue supposed to be called?

Well spotted, I missed out the implementation that's meant to be in 
nsPluginInstancePeerImpl, in my latest patch. I'll attach a new patch below. 
I'll update the test plugin to test it.

4. is this patch actually functioning?

It works for me in both debug and release.

Comment 28

[David Brittain]

Attachment: New complete patch with missing GetValue call

Comment 29

[David Brittain]

Attachment: Updated test that tests GetValue and outputs Hwnd to console

Comment 30

[av (gone)]

Looks better now. I guess we are allowed to add new methods to the 
nsIPluginInstanceOwner interface since it is not plugin API interface. r=av.

David, can you get sr= , drivers permission and check it in?

Comment 31

[David Brittain]

I didn't spot Peter Lubczynski's comment yesterday about the patch in bug 
76085. This does fix the problem that I was having in nsObjectFrame::Paint, so 
that part of the patch is no longer relevant. Do I need to submit a new patch 
after that fix has been checked in, or should I go for sr= as it is?

Comment 32

[av (gone)]

Please submit new patch without changes in nsObjectFrame::Paint and get sr= on 
this. And I guess the fix for 76085 should go in first.

Marking dependency.

Comment 33

[David Brittain]

Attachment: Another patch without the changes in nsObjectFrame::Paint

Comment 34

[David Brittain]

The new patch is no longer dependent on bug 
76085. The first patch in that report has been checked-in and this patch works 
with those changes.

Comment 35

[Marc Attinasi]

David, the patch looks OK, but I have not tested it myself. The tabs seem to be
messed up (use spaces, not tabs, set to 2). What testing has this gottten?
sr=attinasi

Comment 36

[David Brittain]

Attachment: Modified patch with spaces instead of tabs

Comment 37

[David Brittain]

I have tested the patch on Windows 2000 with the a modified version of the test 
plugin (attached above) and with our own plugin which is currently being 
developped.

Comment 38

[Peter Lubczynski]

r=peterl on the new patch, all XP changes nice. We have Marc sr= so I guess now
it needs approvals? 

David, do you have CVS checkin access? If so, request approval by sending mail
to [e-mail], otherwise, Andrei, are you going to check this in?

Comment 39

[av (gone)]

I will as soon as a= is obtained.

Comment 40

[av (gone)]

I guess we should check for mOwner not being zero in all that 
nsPluginInstancePeer methods.

Comment 41

[av (gone)]

Attachment: mOwner check added

Comment 42

[K Carter]

Looks like Asa forgot to record his a=.  This bug shows on his approval tracking
bug 83989.

Comment 43

[Marc Attinasi]

sr=attinasi on the latest path, of course. So, is approval (a=asa) assumed from
that last comment?

Comment 44

[Asa Dotzler [:asa]]

bug 83989 is not an approval.  It is just my tracking bug to make sure that we
don't lose any requests.

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron for trunk checkin (on behalf of drivers)

Comment 46

[av (gone)]

Checked in. Will mark it fixes later, when it passes a couple of cycles on the 
Tinderbox.

Comment 47

[av (gone)]

Marking fixed.

Comment 48

[shrirang khanzode]

verif with testcase url that this works on 0626 trunk/branch builds. 

Comment 49

[Matthew Schultz]

A new bug: http://bugzilla.mozilla.org/show_bug.cgi?id=181138 seems related to
this bug.  Setting the z-index of a layer to show over a flash file does not
work as it still shows it beneath when setting the wmode to transparent.  I have
set up a test attachment at the new posted bug.