From kfm-devel  Mon Apr 19 19:14:53 2010
From: "Maksim Orlovich" <mo85 () cornell ! edu>
Date: Mon, 19 Apr 2010 19:14:53 +0000
To: kfm-devel
Subject: Re: Old Flash? Go upgrade! - New problem with konqueror
Message-Id: <61900.67.246.70.185.1271704493.squirrel () webmail ! cornell ! edu>
X-MARC-Message: https://marc.info/?l=kfm-devel&m=127170453305626
MIME-Version: 1
Content-Type: multipart/mixed; boundary="------=_20100419151453_18408"

------=_20100419151453_18408
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 8bit

> 2010/4/19 Maksim Orlovich <mo85@cornell.edu>:
>>
>>> With some hacking, I indeed get youtube working.
>>
>> Could you elaborate on what exactly you had to do? I am a bit lost here.
>
> Like mentioned kmplayer I had to make sure the plugin creation goes
> sync, had to bypass a few timeout redirects too.

OK. nspluginviewer's startup is embarrassingly sync, so that's one thing I
probably don't have to worry about.

> Also I need the correct src/movie url, not the docbase. The kpart
> constructor gets the width/height attribute, but not the src
> attribute. Modified the test case as
>
> ...
>    flash.setAttribute("type", "application/x-shockwave-flash");
>    flash.setAttribute("width", "400");
>    flash.setAttribute("height", "300");
>    flash.setAttribute("src", "lux.swf");
>    if ((appendFlash = body.appendChild(flash)) && appendFlash.GetVariable)
> {
>                 version = appendFlash.GetVariable("$version");
>    };
>    body.removeChild(flash);
>    document.write(version);
> ..
> (so width/height/src added)

See, this is what I am confused about. Shouldn't the testcase work
w/o an actual flash file (and also w/o width and height)?

The khtml bug I see it that it views the empty string as relative to
document, and hence completes it to the page URL (might be right for empty
but not null, too). I would think the right thing to do would be to pass
an empty string to openUrl? nspluginviewer has some code for that (it just
does the wrong thing).


>>> KHTML also seems to forget calling closeUrl in the test case
>>> 'body.removeChild(flash);'.
>>
>> closeUrl, or destroying the plugin instance entirely, I wonder?
>
> Either way are fine with me.

Please see the attached. I would appreciate if you told me if it
fixes the youtube-still-playing-sound issue for you.

> At the end of the script, the 'flash' variable goes out of scope, so
> no reference to this DOM object. Might be the cause of destruction
> too, dunno.

Well, going out of scope doesn't mean much since garbage collection might
not happen for a long time.

>> This one should be fixed in more recent versions, if my memory serves me
>> right.
>
> My 4.4 checkout is from yesterday. Can the fix be backported then?

Hmm. It should have been fixed by r1077278, which is way older than that.
Might be a different bug hitting the same assert then.


------=_20100419151453_18408
Content-Type: text/x-patch; name="object-not-in-document.diff"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="object-not-in-document.diff"

Index: html/html_objectimpl.cpp
===================================================================
--- html/html_objectimpl.cpp	(revision 1116550)
+++ html/html_objectimpl.cpp	(working copy)
@@ -239,13 +239,20 @@
 void HTMLObjectBaseElementImpl::removedFromDocument()
 {
     document()->underDocNamedCache().remove(m_name, this);
-    HTMLElementImpl::removedFromDocument();
+
+    // When removed from document, we destroy the widget/plugin.
+    // We have to do it here and not just call setNeedComputeContent(),
+    // since khtml will not try to restyle changed() things not in document.
+    clearChildWidget();
+    
+    HTMLPartContainerElementImpl::removedFromDocument();
 }
 
 void HTMLObjectBaseElementImpl::insertedIntoDocument()
 {
     document()->underDocNamedCache().add(m_name, this);
-    HTMLElementImpl::insertedIntoDocument();
+    setNeedComputeContent();
+    HTMLPartContainerElementImpl::insertedIntoDocument();
 }
 
 void HTMLObjectBaseElementImpl::removeId(const DOMString& id)
@@ -364,6 +371,12 @@
         return;
     }
 
+    // Not in document => no plugin.
+    if (!inDocument()) {
+        clearChildWidget();
+        return;
+    }
+
     // Collect information from <param> children for ...
     // It also sometimes supplements or replaces some of the element's attributes
     for (NodeImpl* child = firstChild(); child; child = child->nextSibling()) {
------=_20100419151453_18408--