Bug 45054

Summary: Share more code between HTMLObjectElement and HTMLEmbedElement
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, jamesr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 45049    
Attachments:
Description Flags
Patch dglazkov: review+, commit-queue: commit-queue-

Eric Seidel (no email)
Reported 2010-09-01 13:16:18 PDT
Share more code between HTMLObjectElement and HTMLEmbedElement
Attachments
Patch (29.68 KB, patch)
2010-09-01 13:25 PDT, Eric Seidel (no email) 		dglazkov: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-09-01 13:25:57 PDT
Created attachment 66254 [details] Patch
Dimitri Glazkov (Google)
Comment 2 2010-09-01 13:39:13 PDT
Comment on attachment 66254 [details] Patch This looks great! View in context: https://bugs.webkit.org/attachment.cgi?id=66254&action=prettypatch > WebCore/html/HTMLObjectElement.h:64 > + Extra whitespace here. > WebCore/html/HTMLPlugInElement.cpp:70 > + And here.
Eric Seidel (no email)
Comment 3 2010-09-01 19:14:21 PDT
Whitespace has been cleaned up some in later patches. Gonna see if this cq's clean.
WebKit Commit Bot
Comment 4 2010-09-01 21:30:21 PDT
Comment on attachment 66254 [details] Patch Rejecting patch 66254 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--ignore-tests', 'compositing,media', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 20898 test cases. svg/custom/getsvgdocument.html -> failed Exiting early after 1 failures. 18060 tests run. 370.48s total testing time 18059 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 26 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/3926037
Eric Seidel (no email)
Comment 5 2010-09-01 23:31:41 PDT
Fixing. Will repost for landing shortly.
Eric Seidel (no email)
Comment 6 2010-09-02 01:37:36 PDT
I'm going to land this in smaller pieces. I can't seem to find the regression in the moving code, but I'll make sure all the pieces I land land w/o regression.
Eric Seidel (no email)
Comment 7 2010-09-02 02:10:40 PDT
Landing this in two pieces. First part landed as r66650.
Eric Seidel (no email)
Comment 8 2010-09-02 03:06:32 PDT
Piece 2 as r66653
Eric Seidel (no email)
Comment 9 2010-09-02 03:22:26 PDT
Actually it's going to be 3 pieces. I've reduced the diff causing the failure, now just trying to figure out what part of it is wrong. :)
Eric Seidel (no email)
Comment 10 2010-09-02 03:58:45 PDT
I guess it's going to be 4 pieces. #3 was r66657. I'm down to only like 50 lines left. Still haven't found the bug.
Eric Seidel (no email)
Comment 11 2010-09-02 04:25:07 PDT
Turns out the insanity that is: void HTMLPlugInImageElement::recalcStyle(StyleChange ch) { // FIXME: Why is this necessary? Manual re-attach is almost always wrong. if (!useFallbackContent() && needsWidgetUpdate() && renderer() && !isImageType()) { detach(); attach(); } HTMLPlugInElement::recalcStyle(ch); } needed to be moved from <object> down to the base class too. Who knows why. This code is black magic.
Eric Seidel (no email)
Comment 12 2010-09-02 04:29:36 PDT
Committed r66659: <http://trac.webkit.org/changeset/66659>
Note You need to log in before you can comment on or make changes to this bug.