Bug 48806 - Set frame name before appending it to the frame tree in the Apple Windows, GTK, and EFL ports
Summary: Set frame name before appending it to the frame tree in the Apple Windows, GT...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks: 6751
  Show dependency treegraph
 
Reported: 2010-11-01 16:50 PDT by Daniel Bates
Modified: 2010-11-02 09:04 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.24 KB, patch)
2010-11-01 16:57 PDT, Daniel Bates
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-11-01 16:50:45 PDT
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.
Comment 1 Daniel Bates 2010-11-01 16:57:19 PDT
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).
Comment 2 Martin Robinson 2010-11-01 17:25:11 PDT
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."
Comment 3 Daniel Bates 2010-11-02 09:04:56 PDT
Committed r71120: <http://trac.webkit.org/changeset/71120>