User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030714 Firefox/3.0b4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4) Gecko/2008030714 Firefox/3.0b4 If you visit http://studentindebt.com/flashtest.html with no flash installed: In Firefox 2, this launches the plugin finder service, and all is well. In Firefox 3b4, the plugin finder service is not triggered and the user has to find Flash manually. The markup on the page is very simple, standards compliant, and using the object-only/flash satay method. This is the ONLY way to create a crossbrowser standards compliant embedded flash applet. It's also used with a lot of javascript packages out there. Reproducible: Always Steps to Reproduce: 1. Uninstall Flash 2. Visit http://studentindebt.com/flashtest.html using Firefox 3b4 3. Nothing! 4. Visit same page using Firefox 2 5. Install plugin using plugin finder. 6. ??? 7. Profit! Actual Results: Plugin finder service not triggered. Expected Results: Plugin finder service triggered.

Here's links to the flash uninstaller if you'd like to test this: http://kb.adobe.com/selfservice/viewContent.do?externalId=tn_14157

Added ? for blocking-firefox3. Reason being that if this isn't fixed in firefox 3 but some later version, developers will have to specifically check for firefox 3 and produce alternate markup for firefox 3 specifically (in the same way that we have to produce alternate markup for IE)

The problem here is that GetPluginSupportState is returning ePluginOtherState for this plugin because there is no pluginurl parameter for the object tag. I'm not totally sure why we would need this? ePluginOtherState is commented as "not a plugin at all as far as we can tell"

See also bug 362196

I think this is by design. <object> supports fallback, so that is what we display instead of the plugin. It's just that in this case the fallback (i.e. the contents of the <object>) is empty. Marking non-blocking as this but is likely invalid. That said, this is the second time this has come up. Maybe we should show the plugin finder if there is no fallback at all?

I would be happy enough with showing the plugin finder in the no-fallback case. We have all the infrastructure for this already, I think: see the :-moz-empty-except-children-with-localname pseudo-class, which we can use to detect <object>s that only contain <param> and whitespace.

That would be fine. The key is to have a standards-compliant and consistent way for web designers to trigger the plugin finder service.

I'd suggest raising the issue in the HTML working group. You might want such a way while also providing fallback content...

(In reply to comment #5) > I think this is by design. <object> supports fallback, so that is what we > display instead of the plugin. It's just that in this case the fallback (i.e. > the contents of the <object>) is empty. Perhaps I am missing something, but is it then a bug that if the <object> contains <param name="pluginurl" value=""> then we do display the finder? If there is a fallback should we maybe always display that but still dispatch the plugin not found event so the browser can still show the PFS notification bar? I'm pretty much positive that that is what we do for blocklisted plugins. (In reply to comment #7) > That would be fine. The key is to have a standards-compliant and consistent > way for web designers to trigger the plugin finder service. Is including the pluginurl param a major issue? Doesn't seem like it would be standards in-compliant to me but I don't know how it behaves elsewhere.

I created a second webpage with the pluginurl: http://studentindebt.com/flashtest2.html It seems like it now hits a different issue where it complains about some install-xxx..rdf file. Unfortunately, this method of putting pluginurl in the param tag isn't very well known, and even the developer wiki currently tells the user to use the first method (with the mime-type): http://developer.mozilla.org/en/docs/Using_the_Right_Markup_to_Invoke_Plugins I'll send the html wg an e-mail and see if they can shed some light on this.

(In reply to comment #10) > I created a second webpage with the pluginurl: > http://studentindebt.com/flashtest2.html > > It seems like it now hits a different issue where it complains about some > install-xxx..rdf file. That is just bug 416396

> Perhaps I am missing something Nope. We special-case having a pluginurl attribute or param. > If there is a fallback should we maybe always display that but still dispatch > the plugin not found event The problem is that this will fire for the common <object><embed/></object> case... we don't want that, no matter what.

(In reply to comment #12) > The problem is that this will fire for the common <object><embed/></object> > case... we don't want that, no matter what. Would it be too harsh to fire in the inverse case where we have an object that does *not* have an <embed> as a descendant, even if it has other content?

Some objects also have an <object> as a descendant... But yeah, I suppose I can deal with firing (but not attaching the XBL binding, right?) if the <object> is one we would otherwise process (e.g. doesn't have a classid) and doesn't have an embed/object/applet as a descendant.

A lot of ads do <object><img></object> I don't think we want to show the finder for those either. I think we should only do it if there is no fallback at all to be honest, i.e. only when there is only <param>s and whitespace.

I can certainly live with that. ;)

Boris, what would you think of this then?

Comment on attachment 311695 [details] [diff] [review] Possible fix. >+ if (child->IsNodeOfType(nsINode::eELEMENT) || >+ !child->IsNodeOfType(nsINode::eTEXT)) { >+ hasAlternateContent = PR_TRUE; >+ } else { >+ hasAlternateContent = !child->TextIsOnlyWhitespace(); >+ } How about replacing that with: hasAlternateContent = nsStyleUtil::IsSignificantChild(child, PR_TRUE, PR_FALSE); ? With that, r+sr=bzbarsky.

Boris, mind glancing over the tests here?

Comment on attachment 311744 [details] [diff] [review] Updated fix, with tests. Please replace the ok($1 == $2, "") pattern with is($1, $2, "") (the latter has better error reporting), and looks wonderful.

Comment on attachment 311744 [details] [diff] [review] Updated fix, with tests. >@@ -1733,13 +1736,19 @@ nsObjectLoadingContent::GetPluginSupportState(nsIContent* aContent, > NS_ASSERTION(child, "GetChildCount lied!"); > > if (child->IsNodeOfType(nsINode::eHTML) && >- child->Tag() == nsGkAtoms::param && >- child->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name, >- NS_LITERAL_STRING("pluginurl"), eIgnoreCase)) { >- return GetPluginDisabledState(aContentType); >+ child->Tag() == nsGkAtoms::param) { >+ if (child->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name, >+ NS_LITERAL_STRING("pluginurl"), eIgnoreCase)) { >+ return GetPluginDisabledState(aContentType); >+ } >+ } else if (!hasAlternateContent) { >+ hasAlternateContent = >+ nsStyleUtil::IsSignificantChild(child, PR_TRUE, PR_FALSE); > } Won't we consider almost any <param> as alternate content. We won't fall into the |if| most of the time when name isn't plugin url, and then IsSignificantChild will consider the <param> significant since it considers all elements significant.

If I read the patch right, the new logic is: hasAlternateContent = false; for (each kid) { if (kid is param) { if (kid is pluginurl param) { return stuff; } } else if (!hasAlternateContent) { hasAlternateContent = ...; } } So that looks correct to me in terms of ignoring params for hasAlternateContent purposes...

Marking this regression a blocker, we should get this in once the tree opens.

Fix checked in.