[Webkit-unassigned] [Bug 5793] HTML OBJECT without width/height attributes doesn't honor the size of the image

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=5793


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #72031|review?                     |review-
               Flag|                            |




--- Comment #18 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2010-10-27 07:42:31 PST ---
(From update of attachment 72031)
View in context: https://bugs.webkit.org/attachment.cgi?id=72031&action=review

Overall, this patch looks good, but r- to run layout tests and/or write a new layout test, remove debugging code, and fix the ChangeLog patch.

> WebCore/ChangeLog:1
> -2010-10-27  Adam Roben  <aroben at apple.com>
> +2010-10-27  Julie-Jeongeun-Kim  <jiyuluna at gmail.com>

Please don't delete existing ChangeLog entries.  Your new changelog entry should simply appear before the other ones.

For the most part, your new changelog entry looks good.  Just a couple nits below.

> WebCore/ChangeLog:10
> +        It should let its owner know the real size so that OBJECT is re-layouted.

Let's use "is laid out again." instead of "is re-layouted."

> WebCore/ChangeLog:12
> +        No new tests. (OOPS!)

Did you try to write a test case for this change?

Did you run layout tests to see if any existing layout test results changed?

> WebCore/html/ImageDocument.cpp:46
> +#include <stdio.h>

This is probably left over from debugging.  Please remove it.

> WebCore/html/ImageDocument.cpp:301
> +    IntSize windowSize = IntSize(view->width(), view->height());

Is there a FrameView::size() method that returns an IntSize?

> WebCore/html/ImageDocument.cpp:310
> +                RenderEmbeddedObject* rendererEmbedded = toRenderEmbeddedObject(renderer);
> +                    
> +                if (rendererEmbedded) 

These lines can be combined:

    if (RenderEmbeddedObject* rendererEmbedded = toRenderEmbeddedObject(renderer))

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.