Bug 5256 - Relayout during load causes duplicate plugin part
Summary: Relayout during load causes duplicate plugin part
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Dave Hyatt
URL: http://www.itpolicy.gov.il/wbt/new/le...
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-04 05:06 PDT by mitz
Modified: 2005-12-26 14:08 PST (History)
0 users

See Also:


Attachments
testcase (731 bytes, text/html)
2005-10-08 11:45 PDT, mitz
no flags Details
don't update widget until OBJECT tag is closed (2.90 KB, patch)
2005-10-09 11:15 PDT, mitz
darin: review-
Details | Formatted Diff | Diff
don't update widget until OBJECT tag is closed (2.85 KB, patch)
2005-10-10 15:21 PDT, mitz
no flags Details | Formatted Diff | Diff
don't update widget until OBJECT tag is closed (3.85 KB, patch)
2005-10-14 03:43 PDT, mitz
hyatt: review+
Details | Formatted Diff | Diff
up-to-date diff (3.83 KB, patch)
2005-10-25 00:04 PDT, mitz
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2005-10-04 05:06:37 PDT
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.
Comment 1 Maciej Stachowiak 2005-10-05 21:31:10 PDT
Could this be due to the fallback content bugs (recently fixed)?
Comment 2 mitz 2005-10-05 22:30:38 PDT
(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.
Comment 3 mitz 2005-10-08 11:45:03 PDT
Created attachment 4259 [details]
testcase
Comment 4 mitz 2005-10-08 11:49:26 PDT
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. 
Comment 5 mitz 2005-10-09 11:15:53 PDT
Created attachment 4267 [details]
don't update widget until OBJECT tag is closed
Comment 6 mitz 2005-10-09 11:19:26 PDT
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.
Comment 7 Darin Adler 2005-10-09 13:41:13 PDT
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?
Comment 8 mitz 2005-10-09 13:59:07 PDT
(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.
Comment 9 mitz 2005-10-10 15:21:48 PDT
Created attachment 4301 [details]
don't update widget until OBJECT tag is closed
Comment 10 mitz 2005-10-10 15:25:59 PDT
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.
Comment 11 mitz 2005-10-14 03:43:13 PDT
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.
Comment 12 Dave Hyatt 2005-10-17 20:43:25 PDT
Comment on attachment 4356 [details]
don't update widget until OBJECT tag is closed

r=me
Comment 13 Beth Dakin 2005-10-24 17:47:37 PDT
This patch no longer applies cleanly.
Comment 14 mitz 2005-10-25 00:04:54 PDT
Created attachment 4470 [details]
up-to-date diff

The exact same changes, diffed wrt current versions.
Comment 15 Timothy Hatcher 2005-11-01 10:35:36 PST
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