Bug 48806 – Set frame name before appending it to the frame tree in the Apple Windows, GTK, and EFL ports

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Daniel Bates]

The name of the frame should be set before it's appended to the frame tree in the Apple Windows, GTK, and EFL ports so that the frame count is correct (*) and consistent with the ordering used in the Mac, Qt, and Haiku ports.

(*) Notice, FrameTree::uniqueName() generates unique names using a zero-based frame count. Disregarding the frame name prefix, for a page that that contains two (i.e. FrameTree::m_childCount := 2)  unnamed frames A and B, the unique names would be <!--frame0--> and <!--frame1-->, respectively. For this to work out, the name of the frame must be set before it's appended to the tree (see remark (**)).

(**) Note: Currently, there is no observed difference in the expected results for the Apple Windows, GTK and EFL ports because HTMLFrameElementBase::setName() <http://trac.webkit.org/browser/trunk/WebCore/html/HTMLFrameElementBase.cpp?rev=70976#L160>, which is called before FrameLoaderClient::createFrame(),  generates a unique name with  the correct frame count. And by the implementation of FrameTree::uniqueName() <http://trac.webkit.org/browser/trunk/WebCore/page/FrameTree.cpp?rev=70976#L104>, we don't generate a unique name (with the incorrect child count) if the requested name is non-empty.

[Daniel Bates]

Created attachment 72599 [details]
Patch

At this time we cannot test this change since the observable change is being masked by code in HTMLFrameElementBase::setName() <http://trac.webkit.org/browser/trunk/WebCore/html/HTMLFrameElementBase.cpp?rev=70976#L160> (see comment 1 for more details).

[Martin Robinson]

Comment on attachment 72599 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=72599&action=review

After discussing this with Daniel yesterday, I can confirm that this patch makes a lot of sense.

> WebKit/efl/ChangeLog:10
> +        Mac, Qt, and Haiku ports. In particular, the set the name of the new

I think "the set the name" should probably be "set the name."

[Daniel Bates]

Committed r71120: <http://trac.webkit.org/changeset/71120>