[Webkit-unassigned] [Bug 52594] [Qt] iframe shims don't work

[bugzilla-daemon at webkit.org]

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


Kenneth Rohde Christiansen <kenneth at webkit.org> changed:

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




--- Comment #3 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2011-01-29 15:03:41 PST ---
(From update of attachment 80559)
View in context: https://bugs.webkit.org/attachment.cgi?id=80559&action=review

> Source/WebCore/plugins/PluginView.cpp:1531
> +static void getObjectStack(const RenderObject* ro,
> +                           Vector<const RenderObject*>* roStack)

Keep this one line

> Source/WebCore/plugins/PluginView.cpp:1543
> +static bool checkStackOnTop(
> +        const Vector<const RenderObject*>& iframeZstack,
> +        const Vector<const RenderObject*>& pluginZstack)

Also here.

I do not like the name very much, what about isIframeAbovePlugin or so? that seems more webkitish

> Source/WebCore/plugins/PluginView.cpp:1547
> +    for (size_t i1 = 0, i2 = 0;
> +         i1 < iframeZstack.size() && i2 < pluginZstack.size();
> +         i1++, i2++) {

Are i1 and i2 ever different?

> Source/WebCore/plugins/PluginView.cpp:1584
> +            // Inspect the document order.  Later order means higher
> +            // stacking.

One line

> Source/WebCore/plugins/PluginView.cpp:1609
> +void PluginView::windowCutOutRects(const IntRect& frameRect,
> +                                               Vector<IntRect>& cutOutRects)

One line. I'm not sure cutOut rects is the most descriptive name

> Source/WebCore/plugins/PluginView.cpp:1619
> +    // Get the parent widget

I find the comment a bit redundant

> Source/WebCore/plugins/PluginView.cpp:1624
> +    FrameView* parentFrameView = static_cast<FrameView*>(parentWidget);

There is no toFrameView method?

> Source/WebCore/plugins/PluginView.cpp:1633
> +        const FrameView* frameView =
> +            static_cast<const FrameView*>((*it).get());

one line :-)

> Source/WebCore/plugins/PluginView.cpp:1649
> +                IntPoint point =
> +                    roundedIntPoint(iframeRenderer->localToAbsolute());

I would keep this on one line

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