Bug 5256 – Relayout during load causes duplicate plugin part

• Component: New Bugs
• Status: VERIFIED FIXED

It seems that under certain conditions, a relayout operation during the loading of a page that contains 
plugin content results in a duplicate plugin part being created.

To reproduce: open the URL in Safari. Wait for everything to load. Then choose File > Open Location 
and press the Return key to load the same address.

Expected: a single instance of the Flash content.

Actual: sometimes, (depending on network/server speed?) I get two instances of the Flash content, one 
at the correct position, and another one partially occluding the table to the right.

Looks like during relayout the object needs to move, but for some reason it is recreated in its new 
position and the old instance isn't deleted.

[Maciej Stachowiak]

Could this be due to the fallback content bugs (recently fixed)?

(In reply to comment #1)
> Could this be due to the fallback content bugs (recently fixed)?
> 

FWIW, it happens with TOT as well as with WebKit-416.9.

Created attachment 4259 [details]
testcase

This is a problem with <OBJECT>s that contain an <EMBED>, if layout is forced in the middle of the 
OBJECT but before the EMBED. In the testcase, this is done by displaying the JavaScript alert. The result is 
that both the OBJECT and the EMBED end up being shown. 

Created attachment 4267 [details]
don't update widget until OBJECT tag is closed

Comment on attachment 4267 [details]
don't update widget until OBJECT tag is closed

This patch will be necessary even when bug 5306 is fixed, since it makes no
sense to request plugin content before all parameters are known.

[Darin Adler]

Comment on attachment 4267 [details]
don't update widget until OBJECT tag is closed

Looks pretty good. I suggest that allParamsAvailable() be a const member
function. Also, I think it should be inline since it's just getting the value
of a boolean field. Also, I see no reason for it to be virtual.

How does this work in the case where you dynamically create an object element
and add parameter elements one by one? Or does that not need to work?

(In reply to comment #7)
> How does this work in the case where you dynamically create an object element
> and add parameter elements one by one? Or does that not need to work?

Hm... I'm not sure. I don't think there is (or there should be) a way to say "I'm not done with this" (the 
same questions apply to applet elements, on which this patch is based). Anyway, when bug 5306 is fixed, 
it will be less of an issue.

Created attachment 4301 [details]
don't update widget until OBJECT tag is closed

Comment on attachment 4301 [details]
don't update widget until OBJECT tag is closed

Made allParamsAvailable() non-virtual, const and inline.
I think the other questions are beyond the scope of this bug/patch. Currently,
adding or removing a PARAM triggers an update, but changing the value of a
PARAM doesn't. I don't think it makes sense.

Created attachment 4356 [details]
don't update widget until OBJECT tag is closed

Maciej pointed out that the previous patch would break object elements not
created by the parser. This fixes it.

[Dave Hyatt]

Comment on attachment 4356 [details]
don't update widget until OBJECT tag is closed

r=me

[Beth Dakin]

This patch no longer applies cleanly.

Created attachment 4470 [details]
up-to-date diff

The exact same changes, diffed wrt current versions.

[Timothy Hatcher]

Landed.

Checking in khtml/html/html_objectimpl.cpp;
/cvs/root/WebCore/khtml/html/html_objectimpl.cpp,v  <--  html_objectimpl.cpp
new revision: 1.87; previous revision: 1.86

Checking in khtml/html/html_objectimpl.h;
/cvs/root/WebCore/khtml/html/html_objectimpl.h,v  <--  html_objectimpl.h
new revision: 1.38; previous revision: 1.37

Checking in khtml/html/htmlfactory.cpp;
/cvs/root/WebCore/khtml/html/htmlfactory.cpp,v  <--  htmlfactory.cpp
new revision: 1.9; previous revision: 1.8

Checking in khtml/rendering/render_frames.cpp;
/cvs/root/WebCore/khtml/rendering/render_frames.cpp,v  <--  render_frames.cpp
new revision: 1.83; previous revision: 1.82