can't show plugins that start out hidden

VERIFIED FIXED in mozilla1.2alpha

Status

()

Core
Plug-ins
P2
normal
VERIFIED FIXED
16 years ago
15 years ago

People

(Reporter: Peter Lubczynski, Assigned: Peter Lubczynski)

Tracking

({testcase, topembed+})

Trunk
mozilla1.2alpha
x86
Windows 2000
testcase, topembed+
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PL2:NA] [adt2] [ETA 09/18][verified trunk], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
...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
(Reporter)

Updated

16 years ago
Keywords: testcase

Comment 1

16 years ago
moving to 1.1
Priority: -- → P3
Target Milestone: --- → mozilla1.1alpha

Comment 2

16 years ago
ur testcase works on 0528 brnch now. can u pls double chek ? thx!
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 3

16 years ago
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

16 years ago
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

16 years ago
I bad: this is still broken using the current build
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 6

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

Comment 7

16 years ago
*** Bug 141130 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

16 years ago
Priority: P3 → P2
Whiteboard: [PL2:NA]

Updated

16 years ago
Target Milestone: mozilla1.1alpha → mozilla1.0.2

Comment 8

16 years ago
adding nsbeta1+
Keywords: nsbeta1+
Target Milestone: mozilla1.0.2 → mozilla1.2alpha
(Reporter)

Comment 9

16 years ago
I think this may work on Mac but not on Windows or Linux.
Blocking an embeddor
Keywords: topembed
(Reporter)

Comment 11

16 years ago
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.
Status: REOPENED → ASSIGNED
Keywords: patch
(Reporter)

Comment 12

16 years ago
Created attachment 98064 [details] [diff] [review]
patch v.1

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

Comment 13

16 years ago
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

16 years ago
>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

16 years ago
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
(Reporter)

Comment 16

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

Comment 17

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

Comment 18

16 years ago
>Okay, so create the widget but default it to hidden?
that's right.
cc: waterson who added the code in case he has any feedback
(Reporter)

Comment 20

16 years ago
Created attachment 98146 [details]
better testcase (w/ Flash, QT, Java, & npnull)
(Reporter)

Comment 21

16 years ago
Created attachment 98148 [details] [diff] [review]
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?
Attachment #98064 - Attachment is obsolete: true

Comment 22

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

I like patches like this:) r=serge
Attachment #98148 - Flags: review+

Comment 23

16 years ago
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.
Blocks: 154896
Keywords: edt1.0.2, mozilla1.0.2
Whiteboard: [PL2:NA] → [PL2:NA] [adt2] [ETA 09/11]

Comment 24

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

Comment 25

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

sr=kin@netscape.com as long as it doesn't regress the test cases in bug 37622.
Attachment #98148 - Flags: superreview+

Updated

16 years ago
Keywords: approval, mozilla1.2
Whiteboard: [PL2:NA] [adt2] [ETA 09/11] → [PL2:NA] [adt2] [ETA 09/12]
(Reporter)

Comment 26

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

Comment 27

16 years ago
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.
Peter, can we get this landed on the trunk and bake a bit?
(Reporter)

Comment 29

16 years ago
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

16 years ago
Let's shoot for getting this on branch after 09/13.
Whiteboard: [PL2:NA] [adt2] [ETA 09/12] → [PL2:NA] [adt2] [ETA 09/16]
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.
Keywords: mozilla1.0.2 → mozilla1.0.2-
(Reporter)

Comment 32

16 years ago
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 :(
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 33

16 years ago
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!
Keywords: mozilla1.0.2- → mozilla1.0.2
Whiteboard: [PL2:NA] [adt2] [ETA 09/16] → [PL2:NA] [adt2] [ETA 09/14]
Comment on attachment 98148 [details] [diff] [review]
patch v.2

a=rjesup@wgate.com for 1.0 branch.  Change mozilla1.0.2+ to fixed1.0.2 when
checked in
Attachment #98148 - Flags: approval+

Updated

16 years ago
Keywords: approval, mozilla1.0.2 → mozilla1.0.2+

Updated

16 years ago
Keywords: topembed → topembed+

Comment 35

16 years ago
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!
Keywords: edt1.0.2 → edt1.0.2+
Whiteboard: [PL2:NA] [adt2] [ETA 09/14] → [PL2:NA] [adt2] [ETA 09/18]

Comment 36

16 years ago
verifed fixd on trunk 0916, testcase works great now.
Status: RESOLVED → VERIFIED
Whiteboard: [PL2:NA] [adt2] [ETA 09/18] → [PL2:NA] [adt2] [ETA 09/18][verified trunk]
(Reporter)

Comment 37

15 years ago
patch on branch
Keywords: mozilla1.0.2+ → fixed1.0.2

Comment 38

15 years ago
verified on 0919 branch, works great.
Keywords: fixed1.0.2 → verified1.0.2
You need to log in before you can comment on or make changes to this bug.