315841 - java applets disappear when you have plugin registered with content policy

Status: RESOLVED FIXED

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

Description

[gabe]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1

java dissappers

Reproducible: Always

Steps to Reproduce:
1.instill extenion like adblock
2.go too a site with java

Actual Results:  
java dissappers

Expected Results:  
java should not dissapper

https://bugzilla.mozilla.org/show_bug.cgi?id=309706 stoped java from crashing with adblock but made java not work

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1 ID:2005110923

I see it happen on this site: http://www.jigzone.com/ms/z.php?k=2F4GIDD8
When I disable Adblock, the applet starts working.

Comment 2

[Michael]

Filed regarding bug 309706, comment 58.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

summary:
- wrapping the <object> flushes reflow
- this means that things are flushed by the time ShouldLoad is called
- we won't ever notify when the type of the plugin changes to eType_Plugin (aNotify=false)

fix:
- stop flushing reflow there
- instead, expose a method on nsIObjectLoadingContent:
  void ensurePluginLoaded();
or somesuch

implementation is something like:

   if (mType == eType_Plugin && !GetFrame()) {
     mInstantiating = TRUE;
     RecreateFramesFor(this);
     mInstantiating = FALSE;
     if (!GetFrame())
        return;
   }
   if (!GetFrame()->GEtPluginInstance())
      Instantiate(mContentType, mURI);

presshell can also call this in StartPluginInstance

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

(btw, before bug 309706, this worked purely by chance... the type of the object defaulted to Type_Plugin for embed and applet; and mURI/mContentType was already correct by the time ShouldLoad was called. for <object>, the bug should have already been there.)

Is there an adblock version which works in seamonkey? where can I get adblock?

Comment 5

[gabe]

http://forums.mozillazine.org/viewtopic.php?t=266291&highlight=

it has the latest adblock stuff

Comment 6

[gabe]

 Greasemonkey also casues same problem for some ppl

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

ohh, bz, you'll like this. even if I do fix what I mentioned above, it still doesn't work - checkLoad seems to set an attribute "adblockdata" on the <applet>. This notifies. This constructs frames. -> same problem happens again.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

(in function storeAdblockData, component.js:589)

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch from comment 3

even though this doesn't fully fix the problem, I think we should check it in

Comment 10

[Boris Zbarsky [:bzbarsky]]

> checkLoad seems to set an attribute "adblockdata" on the <applet>.

That's something the content policy API doesn't allow and something adblock needs  to stop doing if it's going to not break.  I've filed bug 316296 on documenting this clearly in nsIContentPolicy.

I'll try to review this later tonight.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 202807 [details] [diff] [review]
patch from comment 3

>Index: content/base/public/nsIObjectLoadingContent.idl
>+   * instantiated. This method does nothing if the type is not #TYPE_PLUGIN.

Why the '#' there?

>Index: content/base/src/nsObjectLoadingContent.cpp
>+nsObjectLoadingContent::EnsureInstantiation()

>+  nsIObjectFrame* frame = GetFrame();

This should really flush out frames first, I think, except if we're called from ObjectURIChanged.  Could we somehow keep track of whether we're in the middle of calling NS_CheckContentLoadPolicy and flush frames if we're not?

>+    // Trigger frame construction

We _really_ don't want to do this from inside ObjectURIChanged.  I guess we don't really need the flush because RecreateFramesFor handles it for us, though.  So I'd say this function should look like:

nsIFrame* frame = GetFrame();
if (!frame) {
  if (! calling NS_CheckContentLoadPolicy) {
    recreate frames;
  }
  if (!frame) {
    return NS_OK;
  }
}

>+  // We may have a plugin instance already; if so, do nothing

Would it make sense for this method to return the nsIPluginInstance?  That's what the caller really wants, after all...  And if this method is noscript it can do that.  Either way, though.

The rest looks good.

Comment 12

[Michael]

Thanks Christian for clarying the cause and Boris for the documentation. Most of the cases have been fixed, and now the video's and java applets load on the trunk builds.

Fixed version:
http://forums.mozillazine.org/viewtopic.php?t=266291

Comment 13

[gabe]

still some java dissppears on some sites

Comment 14

[gabe]

http://www.jchq.net/applets/Exam/Exam.htm


does not work but atlest orginal page i used for url works

Comment 15

[Boris Zbarsky [:bzbarsky]]

Stuff will keep disappearing, possibly, until this bug is actually resolved fixed (that is, until that patch is updated, reviewed, and checked in).

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

OK, addressed those comments. I now always set mInstantiating in ObjectURIChanged. The name for AutoSetToFalse sucks, it is a remnant of my attempt to pass it a PRBool* (which doesn't work for bitfields).

Maybe AutoSetInstantiatingToFalse?

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: Classinfo patch

forgot to include this in the file above

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 204238 [details] [diff] [review]
patch v2

> Maybe AutoSetInstantiatingToFalse?

Yeah, that seems reasonable.  r+sr=bzbarsky.

Comment 19

[gabe]

check this pactch in on trunk

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

I will as soon as I have the time to watch the tree.

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch for checkin

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in content/base/public/nsIObjectLoadingContent.idl;
/cvsroot/mozilla/content/base/public/nsIObjectLoadingContent.idl,v  <--  nsIObjectLoadingContent.idl
new revision: 3.3; previous revision: 3.2
done
Checking in content/base/src/nsObjectLoadingContent.cpp;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v  <--  nsObjectLoadingContent.cpp
new revision: 1.16; previous revision: 1.15
done
Checking in content/base/src/nsObjectLoadingContent.h;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.h,v  <--  nsObjectLoadingContent.h
new revision: 1.6; previous revision: 1.5
done
Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.886; previous revision: 3.885
done
Checking in dom/src/base/nsDOMClassInfo.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMClassInfo.cpp,v  <--  nsDOMClassInfo.cpp
new revision: 1.335; previous revision: 1.334
done

Comment 23

[Philip Withnall (unavailable)]

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

Comment 24

[Jon Kåre Hellan]

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

Comment 25

[Wladimir Palant]

From what I understand this landed on trunk only. Any chance this can make it into 1.8 branch as well? This issue is a major annoyance for Adblock (Plus) users. For example I got a complaint about this page today: http://marriott.com/property/phototour.mi?marshaCode=mowcy&pageID=HWHOM&imageID=1
When any JavaScript content policy is installed this page makes Firefox hang (you can only close tabs, nothing more). Reproducible with Firefox 1.5.0.4 and current Firefox 2.0a3 nightlies, not with trunk nightlies however - most definitely because of this patch.

Comment 26

[Boris Zbarsky [:bzbarsky]]

> From what I understand this landed on trunk only.

Yes, because it modifies the code added in bug 1156, which is trunk-only.  Whatever you're seeing on branch is not this bug (even if the patch in this bug did happen to fix it).  A branch fix, if possible, would have to be totally different.

Comment 27

[Ray Trent]

Hate to say it, but this still happens for me on the latest trunk nightly (2006061405). Exact same symptoms at least (Java applets hang, only when AdBlock installed, e.g. games at pogo.com). Also using the latest AdBlockPlus (0.7.0.2) and Java (build 1.5.0_07-b03). 

Reopening (please pardon me if this is an etiquette faux pas). 

Comment 28

[Boris Zbarsky [:bzbarsky]]

Ray, please file a new bug, with exact steps to reproduce.  It's very likely you're seeing a different issue, in a totally different piece of code, especially since we did check that the patch in this bug fixes the issues this bug was filed on.  For example, bug 336875 had symptoms similar to this one, for totally different reasons.

Please do cc me on the bug you file.

Comment 29

[Gili Tzabari]

FYI: I believe this is related to http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6396012

I feel this is a very severe bug because it takes down the entire browser every time. I can reproduce this bug under FireFox 2.0.0.3 and AdBlock Plus 0.7.2.4 but it is gone if I upgrade to FireFox 3.0 alpha 2. Can't you guys backport the fix to a FireFox 2.x release in the near future? Thank you.

Comment 30

[Marc Bejarano]

(In reply to comment #29)
> Can't you guys backport the
> fix to a FireFox 2.x release in the near future? Thank you.

gili: see comment 26.  file a new bug, please.

Comment 31

[Gili Tzabari]

Filed bug #375070. Thank you.