137231 - can't show plugins that start out hidden

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Peter Lubczynski]

...from bug 129306

Using the CSS visibility property, if a plugin start out hidden, it can never be
made visible.

Testcase:
http://bugzilla.mozilla.org/attachment.cgi?id=72978&action=view

Comment 1

[rubydoo123]

moving to 1.1

Comment 2

[shrirang khanzode]

ur testcase works on 0528 brnch now. can u pls double chek ? thx!

Comment 3

[Sean Kendall Schneyer]

This is still not working for me on Win2K Build 2002053008. I'm not really sure
which builds are considered branch builds, but I've tried the latest builds from
the following directories and they all fails:
/pub/mozilla/nightly/latest
/pub/mozilla/nightly/latest-1.0.0

Comment 4

[rubydoo123]

interesting, this is fixed on the branch builds, reporter could you possibly
download the Netscape7.0 beta release and see if you can repro this?

Comment 5

[rubydoo123]

I bad: this is still broken using the current build

Comment 6

[av (gone)]

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

Comment 7

[rubydoo123]

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

Comment 8

[rubydoo123]

adding nsbeta1+

Comment 9

[Peter Lubczynski]

I think this may work on Mac but not on Windows or Linux.

Comment 10

[Doron Rosenberg (IBM)]

Blocking an embeddor

Comment 11

[Peter Lubczynski]

Okay, I got this working for the default plugin on Windows (and I think other
plugins in general) but I can't seem to get it working for Flash. With Flash,
all I get is a white box. It almost seems like Flash requires the initial stream
to be re-sent AFTER the window is shown as when I right click, I see it says no
movie is loaded. This also doesn't work in Opera, it starts out visible. In any
case, I'll play around with it some more to see if I can get Flash to draw after
it's window is created and displayed for the first time.

Comment 12

[Peter Lubczynski]

Attachment: patch v.1

This patch gets other plugins to work but not Flash. :(

Comment 13

[serge (gone)]

or removing this
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/base/src/nsObjectFrame.cpp&rev=1.357&root=/cvsroot#734
if (IsHidden())
and always create the widget also solves the problem, can we regress?
BTW on unix it actually wrong thing to do do not create a widget for
hidden plugin, e.g if such plugin uses some Xt resources in best case scenario
it can fail, or it'll crash (see bug 90471)

Comment 14

[serge (gone)]

>This patch gets other plugins to work but not Flash. :(
that's because we serve whole stream to the plugin
with window->window==0, and it probably just ignores the data.

Comment 15

[serge (gone)]

or we can do
===================================================================
RCS file: /cvsroot/mozilla/layout/html/base/src/nsObjectFrame.cpp,v
retrieving revision 1.357
diff -u -r1.357 nsObjectFrame.cpp
--- nsObjectFrame.cpp   3 Sep 2002 23:42:52 -0000       1.357
+++ nsObjectFrame.cpp   6 Sep 2002 02:36:25 -0000
@@ -734,7 +734,7 @@
 #ifndef XP_MAC
   // Do not create a widget if 'hidden' (except for Mac, where we
   // always create a widget...)
-  if (IsHidden())
+  if (IsHidden(PR_FALSE))
     return NS_OK;
 #endif

Comment 16

[Peter Lubczynski]

Okay, so create the widget but default it to hidden?

Comment 17

[Jaime Rodriguez, Jr.]

If this is blocking an embeddor, shouldn't it be tagged with "topembed+"?

Comment 18

[serge (gone)]

>Okay, so create the widget but default it to hidden?
that's right.

Comment 19

[Doron Rosenberg (IBM)]

cc: waterson who added the code in case he has any feedback

Comment 20

[Peter Lubczynski]

Attachment: better testcase (w/ Flash, QT, Java, & npnull)

Comment 21

[Peter Lubczynski]

Attachment: patch v.2

This patch removes some old code and that seems to do the trick: the widget is
created but not shown. It appears some plugins like QT and Java still get a
chance to flicker once but I'm not sure anything can be done about this. Flash
doesn't seem to do this. In any case, the Mac was already doing this so the
other platforms should too. Serge, can you use the attached testcase to verify
Linux?

Comment 22

[serge (gone)]

Comment on attachment 98148 [details] [diff] [review]
patch v.2

I like patches like this:) r=serge

Comment 23

[Jaime Rodriguez, Jr.]

beard/kin: can one of you sr= patch v.2. thanks!

This resolves and issue for a major embedoor. Adding edt1.0.2 and mozilla1.0.2
keywords, to nominate this fix for the 1.0 branch.

Comment 24

[Chris Waterson]

I guess if you can verify that this doesn't re-introduce bug 37622, then I have
no qualms with it.

Comment 25

[kinmoz]

Comment on attachment 98148 [details] [diff] [review]
patch v.2

sr=[e-mail] as long as it doesn't regress the test cases in bug 37622.

Comment 26

[Peter Lubczynski]

The testcase in bug 37622, attachment 8115 [details] works fine with this patch because
|GetDesiredSize| returns zero size if we are hidden.

Comment 27

[Olivier Cahagne]

I tested build 20020901 (trunk) on Linux with patch 98148 and tested attachment
98146 [details], which worked fine (Flash 5.0.50, JRE 1.4.0_01, default plug-in, didn't
test QT as I don't have Crossover stuff).
Couldn't test bug 37622 testcase as I don't have any plug-in to play .mid files.

Comment 28

[Doron Rosenberg (IBM)]

Peter, can we get this landed on the trunk and bake a bit?

Comment 29

[Peter Lubczynski]

I'm waiting for the 1.2 alpha freeze to be over. I thought it'd be done Friday
but the list of blocker bugs seems to be growing at the moment. :(

Comment 30

[Jaime Rodriguez, Jr.]

Let's shoot for getting this on branch after 09/13.

Comment 31

[Randell Jesup [:jesup] (needinfo me)]

mozilla1.0.2-; if this won't go onto trunk until 9/13 at earliest this won't
make 1.0.2.  Please re-request for 1.0.3.

Comment 32

[Peter Lubczynski]

patch in trunk, marking FIXED:

/cvsroot/mozilla/layout/html/base/src/nsObjectFrame.cpp,v
new revision: 1.360; previous revision: 1.359

There is no mozilla1.0.3 keyword yet :(

Comment 33

[Jaime Rodriguez, Jr.]

Removing minus from "mozilla1.0.2-", to renominate for the 1.0.2 milestone, as
the 1.0.2 milestone will not close until the middle of next week. Pls reconsider
this patch. Changing ETA to 09/14 for potential branch landing. Thanks!

Comment 34

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 98148 [details] [diff] [review]
patch v.2

a=[e-mail] for 1.0 branch.  Change mozilla1.0.2+ to fixed1.0.2 when
checked in

Comment 35

[Jaime Rodriguez, Jr.]

shrir: pls verify this as fixed on the trunk. thanks!

edt1.0.2+ (per verbal from saari) approval for landing on the 1.0 branch. Pls
land time asap, the replace "mozilla1.0.2+" with "fixed1.0.2". thanks!

Comment 36

[shrirang khanzode]

verifed fixd on trunk 0916, testcase works great now.

Comment 37

[Peter Lubczynski]

patch on branch

Comment 38

[shrirang khanzode]

verified on 0919 branch, works great.