LayoutTests:
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Sep 2006 22:57:04 +0000 (22:57 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Sep 2006 22:57:04 +0000 (22:57 +0000)
        Reviewed by John, Maciej.

        Added test for whether a frame element reports its src attribute as a
        complete, rather than relative, URL.

        * fast/frames/frame-src-attribute-expected.txt: Added.
        * fast/frames/frame-src-attribute.html: Added.
        * fast/frames/resources/frame-src-attribute-subframe.html: Added.

WebCore:

        Reviewed by John, Maciej.

        Integrated some frame and iframe code. I'm trying to fix up frame ownership
        and loading. Reducing the number of different code paths involved
        seemed like a good first step.

        As a side effect, I fixed a bug where FRAME elements would report their
        src attributes as relative, rather than compelete, URLs. (IFRAME elements
        had the correct complete URL behavior.)

        * html/HTMLFrameElement.cpp:
        (WebCore::HTMLFrameElement::isURLAllowed): Fixed comment typo
        (WebCore::HTMLFrameElement::openURL):
            (1) Removed checks that requestFrame does for us
            (2) Added isURLAllowed check, to have one clear bottleneck for it
            (3) Added viewsource check, to have one clear bottleneck for it
        (WebCore::HTMLFrameElement::close): Changed to use the common contentFrame()
        method, instead of finding our content frame in our own unique way.
        (WebCore::HTMLFrameElement::setLocation): Removed isURLAllowed check,
        since openURL does this for us now.
        (WebCore::HTMLFrameElement::src): Return complete URL instead of relative.
        This is what FF does, and it made no sense to have different behaviors
        for FRAME and IFRAME elements.
        * html/HTMLIFrameElement.cpp:
        (WebCore::HTMLIFrameElement::HTMLIFrameElement): Removed duplicate init
        code.
        * html/HTMLIFrameElement.h: Removed src() and openURL() methods, since
        HTMLFrameElement now does everything we need.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16661 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/frames/frame-src-attribute-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/frame-src-attribute.html [new file with mode: 0644]
LayoutTests/fast/frames/resources/frame-src-attribute-subframe.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/HTMLFrameElement.cpp
WebCore/html/HTMLIFrameElement.cpp
WebCore/html/HTMLIFrameElement.h

index 474989737afbbc7d71b331a10fe286fa2b82e655..82d1566785d19fe5fef78936aa91973f6e716d1d 100644 (file)
@@ -1,3 +1,14 @@
+2006-09-29  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by John, Maciej.
+        
+        Added test for whether a frame element reports its src attribute as a
+        complete, rather than relative, URL.
+
+        * fast/frames/frame-src-attribute-expected.txt: Added.
+        * fast/frames/frame-src-attribute.html: Added.
+        * fast/frames/resources/frame-src-attribute-subframe.html: Added.
+
 2006-09-29  David Harrison  <harrison@apple.com>
 
         Reviewed by John Sullivan.
diff --git a/LayoutTests/fast/frames/frame-src-attribute-expected.txt b/LayoutTests/fast/frames/frame-src-attribute-expected.txt
new file mode 100644 (file)
index 0000000..2cb7006
--- /dev/null
@@ -0,0 +1,23 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderFrameSet {FRAMESET} at (0,0) size 800x600
+      RenderFrame {FRAME} at (0,0) size 800x600
+        layer at (0,0) size 798x596
+          RenderView at (0,0) size 798x596
+        layer at (0,0) size 798x596
+          RenderBlock {HTML} at (0,0) size 798x596
+            RenderBody {BODY} at (8,8) size 782x575
+              RenderBlock {P} at (0,0) size 782x18
+                RenderText {#text} at (0,0) size 607x18
+                  text run at (0,0) width 607: "This test checks whether a frame element's 'src' attribute is a complete, rather than relative, URL."
+              RenderBlock {PRE} at (0,34) size 782x15
+                RenderText {#text} at (0,0) size 624x15
+                  text run at (0,0) width 624: "PASS: Frame 'src' attribute should include 'LayoutTests/fast/frames' and does."
+      RenderFrame {FRAME} at (0,-500000) size 0x0
+        layer at (0,0) size 8x8
+          RenderView at (0,0) size 0x0
+        layer at (0,0) size 8x8
+          RenderBlock {HTML} at (0,0) size 0x8
+            RenderBody {BODY} at (8,8) size 0x0
diff --git a/LayoutTests/fast/frames/frame-src-attribute.html b/LayoutTests/fast/frames/frame-src-attribute.html
new file mode 100644 (file)
index 0000000..d23f92e
--- /dev/null
@@ -0,0 +1,27 @@
+<html>\r
+<head>\r
+<script>\r
+function write(s)\r
+{\r
+    frames[0].document.getElementById('console').appendChild(document.createTextNode(s));\r
+}\r
+\r
+function test() {\r
+    var layoutTestDir = location.href;\r
+    layoutTestDir = layoutTestDir.substring(layoutTestDir.indexOf("LayoutTests"), layoutTestDir.lastIndexOf("/"));\r
+\r
+    var frameSrc = document.getElementsByTagName('frame')[1].src;\r
+    if (frameSrc.search(layoutTestDir) != -1)\r
+        write("PASS: Frame 'src' attribute should include '" + layoutTestDir + "' and does.");\r
+    else\r
+        write("FAIL: Frame 'src' attribute should include '" + layoutTestDir + "' but instead is '" + frameSrc + "'.");\r
+}\r
+</script>\r
+</head>\r
+\r
+<frameset onload="test()">\r
+<frame src="data:text/html,<p>This test checks whether a frame element's 'src' attribute is a complete, rather than relative, URL.</p><pre id='console'></pre>">\r
+<frame src="resources/frame-src-attribute-subframe.html">\r
+</frameset>\r
+\r
+</html>\r
diff --git a/LayoutTests/fast/frames/resources/frame-src-attribute-subframe.html b/LayoutTests/fast/frames/resources/frame-src-attribute-subframe.html
new file mode 100644 (file)
index 0000000..e69de29
index 19d2e218b2aef7c598aaf43b141f811813698997..09de2f342a0ac0d965272cb1ea6127b1430e552b 100644 (file)
@@ -1,3 +1,34 @@
+2006-09-29  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by John, Maciej.
+        
+        Integrated some frame and iframe code. I'm trying to fix up frame ownership
+        and loading. Reducing the number of different code paths involved
+        seemed like a good first step.
+        
+        As a side effect, I fixed a bug where FRAME elements would report their
+        src attributes as relative, rather than compelete, URLs. (IFRAME elements
+        had the correct complete URL behavior.)
+
+        * html/HTMLFrameElement.cpp:
+        (WebCore::HTMLFrameElement::isURLAllowed): Fixed comment typo
+        (WebCore::HTMLFrameElement::openURL):
+            (1) Removed checks that requestFrame does for us
+            (2) Added isURLAllowed check, to have one clear bottleneck for it
+            (3) Added viewsource check, to have one clear bottleneck for it
+        (WebCore::HTMLFrameElement::close): Changed to use the common contentFrame()
+        method, instead of finding our content frame in our own unique way.
+        (WebCore::HTMLFrameElement::setLocation): Removed isURLAllowed check,
+        since openURL does this for us now.
+        (WebCore::HTMLFrameElement::src): Return complete URL instead of relative.
+        This is what FF does, and it made no sense to have different behaviors
+        for FRAME and IFRAME elements.
+        * html/HTMLIFrameElement.cpp:
+        (WebCore::HTMLIFrameElement::HTMLIFrameElement): Removed duplicate init
+        code.
+        * html/HTMLIFrameElement.h: Removed src() and openURL() methods, since
+        HTMLFrameElement now does everything we need.
+
 2006-09-30  Nikolas Zimmermann  <zimmermann@kde.org>
 
         Reviewed by Hyatt.
index 244c75a212059047eac111834c908ff2d0f905d9..210feed1d9f7cce06d7570b160e4b11017678473 100644 (file)
@@ -80,7 +80,7 @@ bool HTMLFrameElement::isURLAllowed(const AtomicString &URLString) const
     KURL newURL(document()->completeURL(URLString.deprecatedString()));
     newURL.setRef(DeprecatedString::null);
 
-    // Don't allow more than 1000 total frames in a set. This seems
+    // Don't allow more than 200 total frames in a set. This seems
     // like a reasonable upper bound, and otherwise mutually recursive
     // frameset pages can quickly bring the program to its knees with
     // exponential growth in the number of frames.
@@ -88,7 +88,7 @@ bool HTMLFrameElement::isURLAllowed(const AtomicString &URLString) const
     // FIXME: This limit could be higher, but WebKit has some
     // algorithms that happen while loading which appear to be N^2 or
     // worse in the number of frames
-    if (w->frame()->page()->frameCount() >= 200)
+    if (w->frame()->page()->frameCount() > 200)
         return false;
 
     // We allow one level of self-reference because some sites depend on that.
@@ -109,20 +109,19 @@ bool HTMLFrameElement::isURLAllowed(const AtomicString &URLString) const
 
 void HTMLFrameElement::openURL()
 {
-    FrameView *w = document()->view();
-    if (!w)
+    if (!isURLAllowed(m_URL))
         return;
-    
-    AtomicString relativeURL = m_URL;
-    if (relativeURL.isEmpty())
-        relativeURL = "about:blank";
 
-    // Load the frame contents.
-    Frame* parentFrame = w->frame();
-    if (Frame* childFrame = parentFrame->tree()->child(m_name))
-        childFrame->openURL(document()->completeURL(relativeURL.deprecatedString()));
-    else
-        parentFrame->requestFrame(this, relativeURL, m_name);
+    if (m_URL.isEmpty())
+        m_URL = "about:blank";
+
+    document()->frame()->requestFrame(this, m_URL, m_name);
+
+    // FIXME: This is a relic from how HTMLIFrameElement used to do things.
+    // It's probably unnecessary, since viewsource mode doesn't really work,
+    // and both parseMappedAttribute and attach include the same check.
+    if (contentFrame())
+        contentFrame()->setInViewSourceMode(viewSourceMode());
 }
 
 void HTMLFrameElement::parseMappedAttribute(MappedAttribute *attr)
@@ -226,12 +225,10 @@ void HTMLFrameElement::attach()
 
 void HTMLFrameElement::close()
 {
-    Frame* frame = document()->frame();
-    if (renderer() && frame) {
-        if (Frame* childFrame = frame->tree()->child(m_name)) {
-            childFrame->frameDetached();
-            childFrame->disconnectOwnerElement();
-        }
+    Frame* frame = contentFrame();
+    if (frame && renderer()) {
+        frame->frameDetached();
+        frame->disconnectOwnerElement();
     }
 }
 
@@ -271,9 +268,6 @@ void HTMLFrameElement::setLocation(const String& str)
         return;
     }
     
-    if (!isURLAllowed(m_URL))
-        return;
-
     openURL();
 }
 
@@ -385,7 +379,7 @@ void HTMLFrameElement::setScrolling(const String &value)
 
 String HTMLFrameElement::src() const
 {
-    return getAttribute(srcAttr);
+    return document()->completeURL(getAttribute(srcAttr));
 }
 
 void HTMLFrameElement::setSrc(const String &value)
index a9c6e5f8f69ee50fd2913d674520ca4768d9ed8d..a929f0b8697fad443b23fa3729e0a9588ac70069 100644 (file)
@@ -42,8 +42,6 @@ HTMLIFrameElement::HTMLIFrameElement(Document* doc)
     , needWidgetUpdate(false)
 {
     m_frameBorder = false;
-    m_marginWidth = -1;
-    m_marginHeight = -1;
 }
 
 HTMLIFrameElement::~HTMLIFrameElement()
@@ -161,7 +159,6 @@ void HTMLIFrameElement::detach()
     HTMLElement::detach();
 }
 
-
 void HTMLIFrameElement::recalcStyle( StyleChange ch )
 {
     if (needWidgetUpdate) {
@@ -172,18 +169,6 @@ void HTMLIFrameElement::recalcStyle( StyleChange ch )
     HTMLElement::recalcStyle( ch );
 }
 
-void HTMLIFrameElement::openURL()
-{
-    if (!isURLAllowed(m_URL))
-        return;
-    if (m_URL.isEmpty())
-        m_URL = "about:blank";
-    
-    document()->frame()->requestFrame(this, m_URL, m_name);
-    if (contentFrame())
-        contentFrame()->setInViewSourceMode(viewSourceMode());
-}
-
 bool HTMLIFrameElement::isURLAttribute(Attribute *attr) const
 {
     return attr->name() == srcAttr;
@@ -209,11 +194,6 @@ void HTMLIFrameElement::setHeight(const String &value)
     setAttribute(heightAttr, value);
 }
 
-String HTMLIFrameElement::src() const
-{
-    return document()->completeURL(getAttribute(srcAttr));
-}
-
 String HTMLIFrameElement::width() const
 {
     return getAttribute(widthAttr);
index 07e60d7b6f42457615cc080f6a22249bc9cd3121..1b4c3d66fd8d4b81dac79d55f6c7cca3ab22c913 100644 (file)
@@ -63,11 +63,7 @@ public:
     String width() const;
     void setWidth(const String&);
 
-    virtual String src() const;
-
 protected:
-    virtual void openURL();
-
     bool needWidgetUpdate;
 
  private: