Closed Bug 311172 Opened 17 years ago Closed 16 years ago

[FIX]Firefox crash when Flash disabled via userContent.css [@ nsObjectFrame::PluginNotAvailable ]


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

1.8 Branch


(Not tracked)



(Reporter: elmar.ludwig, Assigned: bzbarsky)




(5 keywords)

Crash Data


(2 files)

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b5) Gecko/20051003 Firefox/1.4.1

Firefox crashes for me immediately after loading with
a current Firefox 1.5 branch build on Linux, but only when Flash is blocked via
the following code in chrome/userContent.css:

object[type="application/x-shockwave-flash"] {
    display: none !important;

When I remove this code from userContent.css, no crash occurs. Note that I am
not using the Flashblock extension (the only installed extension is ChatZilla)
unlike the very similar bug 282933. I filed a new report because this occurs
without Flashblock and my stack trace is different (though this may be related
to code changes from e.g. bug 1156). I also do not see a crash on the URL in
bug 282933, but the content may have changed since that bug was reported in
February 2005.

I've also tested some of the flash demos from this site:

None of them crash for me.

Talkback ID: TB10162428W

line 652]
line 2681]
line 3837]
line 3770]
line 3443]
line 119]
line 753]
line 921]
line 856]
line 385]
line 344]
line 689]
Attached file testcase
Minimal testcase that still crashes for me.
Keywords: testcase
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051004
Firefox/1.4.1 ID:2005100419

I see the same in windows. It crashes only with plugin.default_plugin_disabled
to true.

Regression between 1.8b2_2005062912 and 1.8b2_2005062922.
This leaves the patch for bug 277434 as the most likely candidate, I guess...

Looking at a build just before 20050629 it seems to me that the CSS rule from
userContent.css just doesn't match (since the <embed> tag is missing the type
attribute), so maybe the change in bug 277434 has just uncovered an older bug.

cc'ing jst as he might have an idea what is wrong here.
OS: Linux → All
Not seeing this on the trunk.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051207 Firefox/1.6a1

No crash on either trunk or branch, but on branch the Flash still shows up.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

The minimal testcase still crashes for me on the branch (Firefox 1.5 final).
It does not crash on the trunk, as Mark already mentioned im comment 6.
Elmar, is it the same stack still? I still can't seem to get the userContent.css rules to prevent Flash from showing in branch...
Yes, exactly the same stack as before:

TB12761954H  (Firefox 1.5 final)
TB12762284K  (latest branch build 2005120804)

Hiding Flash via userContent.css works fine for me here on the branch (apart from the cases where it crashes the browser, of course).
So.... On the one hand I cannot reproduce this crash on branch (1.8.0 Firefox build).  Not only can I not reproduce it, but it's not obvious to me how that testcase could crash, given no extensions installed.

On the other hand, it's trivial to write a crash testcase here -- just have an event handler for the event we fire rearrange the DOM (which will kill the frame).  With any luck we will then crash when we set mIsBrokenPlugin or mState.  If we don't, we'll call mContent->GetDocument() which is virtual.  Not good.

The same issues exist on the aviary branch, of course; not sure how much we care about that.

Flags: blocking1.8.1?
Flags: blocking1.8.0.3?
I haven't really tested this, largely because I have no idea how.  Do we have testcases for this plugin finder stuff somewhere?

Elmar, does this patch fix the crash for you?
Attachment #218499 - Flags: superreview?(jst)
Attachment #218499 - Flags: review?(jst)
Attachment #218499 - Flags: approval1.8.0.3?
Attachment #218499 - Flags: approval-branch-1.8.1?(jst)
We don't have textcases really - I usually just uninstall flash and visit a flash page and check that it installs correctly and all.
That's not going to cut it.  For me to have any confidence in landing this on a stable branch (which needs to happen), I need to test it against some testcases that exercise all major aspects of the plug-in finder stuff (at the least, I need to verify that there are no changes in look or behavior, on pages that use all three of object/embed/applet, on pages that do and do not set type attributes, and for both plug-ins that get a URI and plug-ins that do not).  This involves knowing what the plug-in finder is supposed to behave like, by the way (or rather which behaviors absolutely need to be tested).  Is that documented anywhere?

I can't believe we've been landing changes to this code on "stable" branches without having such tests, and I'd really appreciate if the original authors of the code would point to the tests they used to test their stuff before landing it; ideally said stuff would be in a relevant CVS test directory.  Let's do that while said original authors are still around, so we don't have to reverse-engineer this code a few years hence like we're having to do for so much code right now.
Keywords: qawanted
I have some testcases back at work, biesi perhaps has some for his XBL work (which might be the most affected by this change).  I guess toolkit/mozapps/plugins/tests would be a decent location.

There is no design document for the wizard UI, as I wrote the original stuff late during the aviary release cycle.  I could write something on the wiki and then QA could use it to build/write whatever they need.

plugin tests should live in plugins or layout, since they are still valid whether or not we have some specific null plugin.
layout/html/tests would be a better location, imho, since we're testing layout code.

That is, the tests should live in layout; the descriptions of how the wizard behaves in response to those tests should live in mozapps as you said.  The relationship should be documented in a README in the tests dir.

> There is no design document for the wizard UI

Could one be written please?  Better late than never.

Note that imo this is a must for, so anything that could be done to expedite me being able to test this and land it so it doesn't miss would be much appreciated...
Comment on attachment 218499 [details] [diff] [review]
Patch for the issue I do see

>+    // Make sure to fire the event AFTER we've finished touching out members.


Attachment #218499 - Flags: superreview?(jst)
Attachment #218499 - Flags: superreview+
Attachment #218499 - Flags: review?(jst)
Attachment #218499 - Flags: review+
Attachment #218499 - Flags: approval-branch-1.8.1?(jst)
Attachment #218499 - Flags: approval-branch-1.8.1+
Assignee: nobody → bzbarsky
Priority: -- → P1
Summary: Firefox crash when Flash disabled via userContent.css [@ nsObjectFrame::PluginNotAvailable ] → [FIX]Firefox crash when Flash disabled via userContent.css [@ nsObjectFrame::PluginNotAvailable ]
Target Milestone: --- → mozilla1.8.1beta1
(In reply to comment #12)
> Elmar, does this patch fix the crash for you?

I'm fairly busy this week and building from source takes time. I will see what I can do though. is my current tests.

Four of them:
  - first tests a plugin that doesn't exist
  - second tests flash in embed
  - third tests flash in object
  - fourth tests flash/real in embed

I tried with the patch and see no behavior change. Note that it is best to open each test in a new tab, since we have a regression and aren't clearing the missing plugins list for each tab anymore on the 1.8 branch and trunk.
Awesome.  Thanks!  Checked in on the 1.8 branch.

Doron, is there a bug for the 1.8/trunk issue you mention in comment 21?
Closed: 16 years ago
Keywords: qawantedfixed1.8.1
Resolution: --- → FIXED
Bug 334674 is about the tabs not clearing their missing plugin data.
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.3?
Flags: blocking1.8.0.3+
Comment on attachment 218499 [details] [diff] [review]
Patch for the issue I do see

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #218499 - Flags: approval1.8.0.3? → approval1.8.0.3+
Fixed on 1.8.0 branch.
Keywords: fixed1.8.0.3
verified with 2.0b2 builds from 20060821
Crash Signature: [@ nsObjectFrame::PluginNotAvailable ]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.