[Webkit-unassigned] [Bug 14845] Frame's noResize attribute can not be set by JavaScript

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #87056|review?                     |review+
               Flag|                            |




--- Comment #19 from Darin Adler <darin at apple.com>  2011-03-28 08:41:35 PST ---
(From update of attachment 87056)
View in context: https://bugs.webkit.org/attachment.cgi?id=87056&action=review

OK as-is. Suggestions for further improvement.

> Source/WebCore/html/HTMLFrameElement.cpp:90
> +        m_noResize = !attr->isNull(); // This will evaluate to false when frame.noResize := false, which is what we want.

The reason to cache the existence of an attribute in a boolean local is performance; I do not think this code is performance critical and hence I don’t think m_noResize is a good idea.

I suggest we make the noResize() function non-inline and change it to call hasAttribute. This eliminates the need for an m_noResize data member.

I don’t understand the syntax "when frame.noResize := false" so I don’t know what your comment is saying.

> Source/WebCore/html/HTMLFrameElement.cpp:95
> +        if (HTMLFrameSetElement* frameSetElement = containingFrameSetElement(this)) {
> +            RenderObject* frameSetRenderer = frameSetElement->renderer();
> +            if (frameSetRenderer && frameSetRenderer->isFrameSet())
> +                toRenderFrameSet(frameSetRenderer)->computeEdgeInfo();
> +        }

The division of labor here seems wrong. The RenderFrame is the class that knows that edge info is based on noResize and hasFrameBorder, but now the HTML frame element is responsible for calling directly to the RenderFrameSet. I don’t think that RenderFrame should be half responsible for this. Instead it should be entirely responsible.

I think that here in HTMLFrameElement we should be calling updateFromRenderer on RenderFrame and RenderFrame in turn should be turning around and calling the RenderFrameSet to say that its edge info changed and then the RenderFrameSet, based on the fact that a frame called to say its edge info changed, will decide to recompute the edge info.

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