129306 - invisible layer not hiding flash content

Status: VERIFIED FIXED

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

Description

[Sean Kendall Schneyer]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8+)
Gecko/20020301
BuildID:    2002030103

When viewing http://www.dfwairport.com/ a fake pop-up ad is displayed using
layers. When you close the ad, the border dissappears (becomes invisible) as
expected, however the flash portion becomes embedded within the page wherever
the ad is closed.

Reproducible: Always
Steps to Reproduce:
1. Go to http://www.dfwairport.com/
2. Move the "pop-up" ad around the screen.
3. Close the ad by clicking on the "X".
4. Watch the flash portion of the pop-up get embedded in the page

Actual Results:  The flash portion of the ad does not dissappear when the layer
is made invisible.

Expected Results:  The entire pop-up should be made invisible.

I wans't sure which component this really belonged to since there are so many
things to choose from (plug-in, javascript, etc).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Compositor.  We already have bugs on this, iirc.  See also bug 44993

Comment 2

[Sean Kendall Schneyer]

I just looked through the 108 open Compositor bugs and didn't find anything
matching the one that I reported. Also, the problem described in bug 44993
appears to be fixed. The test case for that bug worked fine for me. Perhaps
there is a problem with programatically changing a plug-in from an visible state
to a invisible state?

If I get some time later this week, I'll see if I can put together a testcase.

Comment 3

[David Pedowitz]

Attachment: simple test case

Confirming Win2k Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8)
Gecko/20020204

Comment 4

[David Pedowitz]

Attachment: Updated test case with text

Sorry for the dual attachments, I added some text to the layer... you'll see
the text disappear and reappear but the swf never goes away....

Comment 5

[Sean Kendall Schneyer]

Attachment: Test case with <DIV> starting as hidden.

Here is the same testcase with the <DIV> starting out as hidden. Here you see
that the flash object is hidden along with the text, but when you try to make
it visible only the text shows up. 

So, basically if the plug-in start out as hidden, it stays hidden. If it starts
out visible, it stays visible.

Comment 6

[Sean Kendall Schneyer]

Any chance of this ever becoming a confirmed bug? The test cases clearly show
that there is a problem.

Comment 7

[Alfonso Martinez]

Confirming with win98, build 2002032803

Comment 8

[Kevin McCluskey (gone)]

Reassigning to peterl. Appears to be plugin issue.

Comment 9

[Peter Lubczynski]

Yup, looks like in nsObjectFrame::DidReflow we unconditionally show the view

Comment 10

[Peter Lubczynski]

Attachment: patch to show/hide view accordingly

this patch fixes showing/hiding plugins through CSS visibility

Comment 11

[Peter Lubczynski]

Comment on attachment 78809 [details] [diff] [review]
patch to show/hide view accordingly

Okay, this patch needs a little work:
First, it doesn't work on Mac at all.
Second, doesn't work if plugin is initially hidden

From problem #1, I don't know how to "hide" a plugin on Mac. Simply cutting off
it's Paint() isn't enough to stop animations. Killing the timer does do the
trick, but then I can't restart it :( 

From problem #2, our code only creates a view and a widget at startup. We'll
have to somehow do this later if we detect we're visible but have no view.

Hm...second thought...maybe we should go with this pretty low-risk patch as it
is that at least lets you hide plugins on Windows and open new bugs on both 
these more complicated issues. Thoughts?

Comment 12

[Marc Attinasi]

Comment on attachment 78809 [details] [diff] [review]
patch to show/hide view accordingly

Patch looks fine to me, but I cannot comment on the approach you suggest. I
guess fixing this as a Windows fix, and filing a bug for the Mac fix is OK.

sr=attinasi (cleared the needs work checkbox too)

Comment 13

[av (gone)]

Comment on attachment 78809 [details] [diff] [review]
patch to show/hide view accordingly

OK, having it on Windows is better than nothing, r=av

Comment 14

[Peter Lubczynski]

Patch in trunk, marking FIXED.

New bugs open: 
(Mac) http://bugzilla.mozilla.org/show_bug.cgi?id=137230
(testcase #3) http://bugzilla.mozilla.org/show_bug.cgi?id=137231

Since this effects CSS visibility style, do we want to take this on the 1.0
branch? If so, please nominate with appropriate keywords.

Comment 15

[Sean Kendall Schneyer]

Sorry to take so long to check up on this, but this bug isn't completely fixed.
The test case in comment #4 works fine, however, the test case that I included
in comment #5 still doesn't work.

I'm currently using Build 2002041503 on Win2K. I didn't reopen this bug to give
others a chance to verify that I'm not just missing something here.

Comment 16

[Sean Kendall Schneyer]

Okay, I'm obviously braindead. I now see that a new bug has already been opened
for the scenario I pointed out. Sorry for the extra spam. 

I can confirm that the other part of the bug is actually fixed on Win2K Build ID
2002041503.

Comment 17

[rubydoo123]

this is used quite often with dynamic HTML and could be a high exposure issue

Comment 18

[Jaime Rodriguez, Jr.]

Chris - Can you verify this is fixed, and does not cause any other regressions?

Comment 19

[Gert Jørgensen]

Maybe I'm not so smart, but it seems to me that it still hasn't been fixed in
the 2002041711 build (moz 1.0 RC1) if I try out the "simple test case"..

Or have you guys just assigned the bugs for someone else to fix ?

Comment 20

[Chris Petersen]

Ok, I can verify that both first and second test case work in the Windows April
18th trunk build (2002-04-18-03). The third test case appears just like comment
Perhaps we create a new bug for that one ?

Comment 21

[Chris Petersen]

I meant to say,  Comment 5 is still occuring for me.

Comment 22

[scottputterman]

adding adt1.0.0-.  Let's look at this again for RTM.  Raising impact for the
moment.  How prevalent are pop flash ads?

Comment 23

[Chris Petersen]

Ok, I see that there is a bug for comment 5. Marking this bug verified fixed.

Comment 24

[Peter Lubczynski]

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

Comment 25

[/\/\arcio Galli]

Marking topembed.  This is important for evangelism. This is also one
requirement for embeded products. Here are two reasons that is clear now: (1)
Advertising industry, specifically Rich Media vendors, can use their ad content
and control style positioning and visibility. (2) We have one major topsite in
from the Latin America evangelism effort (bank). 

Comment 26

[Peter Lubczynski]

Removing minus from adt1.0.0 to renominate.

Jud, does this need a topembed+?

The patch in this bug is also needed for bug 120875.

Comment 27

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

Comment on attachment 78809 [details] [diff] [review]
patch to show/hide view accordingly

a=[e-mail] (make sure it's in trunk too)

Comment 28

[scottputterman]

adding adt1.0.0+ for checkin to the 1.0 branch.

Comment 29

[Peter Lubczynski]

patch in branch

thanx!

Comment 30

[Chris Petersen]

Verified on Windows Me May 22 branch build (2002-05-22-08).

Comment 31

[Peter Lubczynski]

Reopening, this bug has reappeared on Win32 in 1.4a. Works on OSX.

Comment 32

[Peter Lubczynski]

Attachment: updated testcase

Comment 33

[Peter Lubczynski]

This is likely a regression from bug 157445.

Comment 34

[Peter Lubczynski]

Attachment: patch for new problem

Here's a patch that adds some virtual SupportsVisibilityHidden lovin' to the
object frame like was done for the ListControlFrame in bug 157445.

Comment 35

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 119868 [details] [diff] [review]
patch for new problem

<sigh>.  Yeah, we need this.  :(

Comment 36

[Peter Lubczynski]

checked into trunk, marking FIXED.

Comment 37

[Greg K.]

Actually, using FizzillaMach/2003040809, the hidden Flash object shows up on
mousing over the "Create," "See," and "About" buttons.

Comment 38

[Bob Clary [:bc] (inactive)]

flash on  http://msnbc.com/news/default.asp is hidden when a menu is opened with
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030411 and 
File name: NPSWF32.dll Shockwave Flash 6.0 r79

Comment 39

[bmartin]

Marking Verified Fixed

testcase in Comment #5 worked as expected in latest Mozilla (ID: 2003051204)