[webkit-reviews] review requested: [Bug 21638] WebCore/page/FrameTree.cpp:find() dispatches form submissions to incorrect frame : [Attachment 24681] Revised Fix for FrameTree.cpp for ambiguous frames.

Sun Oct 26 04:16:06 PDT 2008

John Holdsworth <webkit at johnholdsworth.com> has asked  for review:
Bug 21638: WebCore/page/FrameTree.cpp:find() dispatches form submissions to
incorrect frame
https://bugs.webkit.org/show_bug.cgi?id=21638

Attachment 24681: Revised Fix for FrameTree.cpp for ambiguous frames. 
https://bugs.webkit.org/attachment.cgi?id=24681&action=edit

------- Additional Comments from John Holdsworth <webkit at johnholdsworth.com>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 37894)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,17 @@
> +2008-10-26  John Holdsworth	<webkit at johnholdsworth.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Tests: fast/frames/frameTree-top.html
> +		  fast/frames/frameTree-frame.html
> +		  fast/frames/frameTree-form.html
> +
> +	   Frames now searched for by name starting at the innermost frame
> +	   moving up the tree to the outermost containing parent level by
level.
> +
> +	   * page/FrameTree.cpp:
> +	   (WebCore::FrameTree::find):
> +
>  2008-10-25  Geoffrey Garen  <ggaren at apple.com>
>  
>	   Not reviewed.
> Index: WebCore/page/FrameTree.cpp
> ===================================================================
> --- WebCore/page/FrameTree.cpp	(revision 37894)
> +++ WebCore/page/FrameTree.cpp	(working copy)
> @@ -183,10 +183,15 @@ Frame* FrameTree::find(const AtomicStrin
>      if (name == "_blank")
>	   return 0;
>  
> -    // Search subtree starting with this frame first.
> -    for (Frame* frame = m_thisFrame; frame; frame =
frame->tree()->traverseNext(m_thisFrame))
> -	   if (frame->tree()->name() == name)
> -	       return frame;
> +    // Search up tree starting with this frame first.
> +    for (Frame* parent = m_thisFrame; parent; parent =
parent->tree()->parent()) {
> +	   for (Frame* frame = parent; frame; frame =
frame->tree()->traverseNext(parent)) {
> +	      if (frame->tree()->name() == name)
> +		   return frame;
> +	   }
> +    }
> +
> +    // page search may no longer be required...
>  
>      // Search the entire tree for this page next.
>      Page* page = m_thisFrame->page();
> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 37894)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,14 @@
> +2008-10-26  John Holdsworth	<webkit at johnholdsworth.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Frames now searched for by name starting at the innermost frame
> +	   moving up the tree to the outermost containing parent level by
level.
> +
> +	   * fast/frames/frameTree-form.html: Added.
> +	   * fast/frames/frameTree-frame.html: Added.
> +	   * fast/frames/frameTree-top.html: Added.
> +
>  2008-10-24  Eric Seidel  <eric at webkit.org>
>  
>	   Reviewed by Sam Weinig.
> Index: LayoutTests/fast/frames/frameTree-form.html
> ===================================================================
> --- LayoutTests/fast/frames/frameTree-form.html	(revision 0)
> +++ LayoutTests/fast/frames/frameTree-form.html	(revision 0)
> @@ -0,0 +1,6 @@
> +<!-- see/open frameTree-top.html -->
> +<html><body>
> +<form target=out action='http://www.google.com/search'>
> +Search for: <input name='q' value='webkit'>
> +<input type=submit value='Click me'></form>
> +Output from form submission should appear below.
> Index: LayoutTests/fast/frames/frameTree-frame.html
> ===================================================================
> --- LayoutTests/fast/frames/frameTree-frame.html	(revision 0)
> +++ LayoutTests/fast/frames/frameTree-frame.html	(revision 0)
> @@ -0,0 +1,6 @@
> +<!-- see/open frameTree-top.html -->
> +<html>
> +<frameset rows='100,*'>
> +	<frame src='frameTree-form.html'>
> +	<frame src='javascript: ""' name='out'>
> +</frameset>
> Index: LayoutTests/fast/frames/frameTree-top.html
> ===================================================================
> --- LayoutTests/fast/frames/frameTree-top.html	(revision 0)
> +++ LayoutTests/fast/frames/frameTree-top.html	(revision 0)
> @@ -0,0 +1,10 @@
> +<--
> +
> +Open to test fix for bugzilla Bug# 21638: WebCore/page/FrameTree.cpp:find()
dispatches form submissions to incorrect frame
> +
> +-->
> +<html>
> +<frameset cols='50%,50%'>
> +<frame src='frameTree-frame.html'>
> +<frame src='frameTree-frame.html'>
> +</frameset>