Bug 45054 - Share more code between HTMLObjectElement and HTMLEmbedElement
Summary: Share more code between HTMLObjectElement and HTMLEmbedElement
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
Depends on:
Blocks: 45049
  Show dependency treegraph
Reported: 2010-09-01 13:16 PDT by Eric Seidel (no email)
Modified: 2010-09-02 04:29 PDT (History)
3 users (show)

See Also:

Patch (29.68 KB, patch)
2010-09-01 13:25 PDT, Eric Seidel (no email)
dglazkov: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-09-01 13:16:18 PDT
Share more code between HTMLObjectElement and HTMLEmbedElement
Comment 1 Eric Seidel (no email) 2010-09-01 13:25:57 PDT
Created attachment 66254 [details]
Comment 2 Dimitri Glazkov (Google) 2010-09-01 13:39:13 PDT
Comment on attachment 66254 [details]

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.
Comment 3 Eric Seidel (no email) 2010-09-01 19:14:21 PDT
Whitespace has been cleaned up some in later patches.  Gonna see if this cq's clean.
Comment 4 WebKit Commit Bot 2010-09-01 21:30:21 PDT
Comment on attachment 66254 [details]

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
Comment 5 Eric Seidel (no email) 2010-09-01 23:31:41 PDT
Fixing.  Will repost for landing shortly.
Comment 6 Eric Seidel (no email) 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.
Comment 7 Eric Seidel (no email) 2010-09-02 02:10:40 PDT
Landing this in two pieces.  First part landed as r66650.
Comment 8 Eric Seidel (no email) 2010-09-02 03:06:32 PDT
Piece 2 as r66653
Comment 9 Eric Seidel (no email) 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. :)
Comment 10 Eric Seidel (no email) 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.
Comment 11 Eric Seidel (no email) 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()) {

needed to be moved from <object> down to the base class too.  Who knows why.  This code is black magic.
Comment 12 Eric Seidel (no email) 2010-09-02 04:29:36 PDT
Committed r66659: <http://trac.webkit.org/changeset/66659>