347639 - Dynamically created flash with application/octet-stream show up as plugin placeholder

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Martijn Wargers (dead)]

See url, you see plugin placeholders where you should see some flash text.
I'll attach a testcase to this bug that shows the bug.

Comment 1

[Martijn Wargers (dead)]

Attachment: flash needed for testcase, with application/octet-stream mimetype

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Two embeds in the testcase, one directly in the page, one dynamically created. 
The dynamically created one gives the plugin placeholder, that should not happen.
This works fine on current branch builds.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Biesi: This sounds likely related to your rewrite.

Comment 4

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

hmm. this is an interesting one. We start loading the object when you the src attribute is set, and find out that we can't handle application/octet-stream.

We ignore changes to the type attribute after we started loading the object.

Unfortunately I can't reproduce this on the original site, but bug 347736's patch may significantly reduce the impact of this.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is there any point in starting the load before we're in the document? I know we do for images, but that is partially web-compat to allow pages to preload images.

Comment 6

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

why wait until we're inserted into the doc?

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Can we deal with all types of chages, such as to the type attribute, if we've already started loading? There's also the issue that we could get a different baseURI when inserted into a parent.

Comment 8

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

we handle the base URI change correctly, we do check for a changed URI when the plugin is inserted into the doc. we could extend that to types I suppose...

Comment 9

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

Attachment: patch

jonas convinced me to only start loads once we get inserted into a document.

note that if we start the load before that, and it is a plugin, then we will not really be able to use that data because we don't have a frame yet, so we'd have to start the load again. waiting until we get inserted avoids that problem.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272908 [details] [diff] [review]
patch

Hmmm.  Yeah, I guess this makes sense...

If you wanted to, you could also add a check for whether DoneAddingChildren() got called here.  Might not matter, though.

Comment 11

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

hm... is it really possible that aNotify is true, mDoneAddingChildren false, and SetAttr is being called?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yes, of course.  Consider an <object> with a <script> child which sets attributes on that <object>.  Pretty stupid thing to do, but certainly possible. ;)

Comment 13

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

Attachment: patch v2

OK, check mIsDoneAddingChildren

Comment 14

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

Checking in content/html/content//src/nsHTMLObjectElement.cpp;
/cvsroot/mozilla/content/html/content/src/nsHTMLObjectElement.cpp,v  <--  nsHTMLObjectElement.cpp
new revision: 1.104; previous revision: 1.103
done
Checking in content/html/content//src/nsHTMLSharedObjectElement.cpp;
/cvsroot/mozilla/content/html/content/src/nsHTMLSharedObjectElement.cpp,v  <--  nsHTMLSharedObjectElement.cpp
new revision: 1.87; previous revision: 1.86
done