Bug 56109

Summary: CSS borders for iframes don't show up
Product: [Applications] konqueror Reporter: jon perez <jbperez808>
Component: khtml rendererAssignee: Konqueror Developers <konq-bugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Slackware   
OS: Linux   
Latest Commit: Version Fixed In:

Description jon perez 2003-03-18 21:07:10 UTC
Version:            (using KDE KDE 3.1KDE 3.0.3)
Installed from:    Slackware PackagesSlackware Packages
OS:          Linux

Clicking the button should immediately load in new.html into the IFRAME but it doesn't.  Sometimes clicking on the IFRAME area will cause the new source to be loaded in but not always.

This is actually a problem with Konqurer 3.0.1, but I have no time to test on the latest Konqueror.  Considering it has not been mentioned in the list of IFRAME bugs, it may likely still be there.

Another problem which I should actually be filing a separate bug report for is the fact that the border on the IFRAME does not show up even though the style specifies it should.  Sorry, but I am in a bit of a pinch here...

<html>
<head>
<script>
<!--
  function change() {
    tempFrame=document.getElementById("if");
    tempFrame.src="new.html"
  }
-->
</script>
</head>
<body>
<input type="button" value="change" onclick="change()"><br />
<iframe id="if" src="old.html" style="border:solid 4px"></iframe>
</body>
</html>
Comment 1 Maksim Orlovich 2003-05-12 20:57:08 UTC
iframe.src loading seems to work fine in HEAD, the border isn't there, though 
Comment 2 Allan Sandfeld 2005-02-22 19:02:06 UTC
Can almost be done by adding "bool canHaveBorder() { return true;}" to RenderPart, but should be done so the current BORDER attribute is rendered using CSS and so that the border is updated when exposed, which it currently won't be.
Comment 3 Germain Garand 2006-06-27 06:31:03 UTC
SVN commit 555345 by ggarand:

. Correct drawing of borders on iframes and objects (#118277/#56109)
. Frameborder attribute for iframes (http://www.w3.org/TR/html401/present/frames.html#adef-frameborder)
. Fix widget counter-mask not being updated in time, thus sometimes missing a repaint (e.g: http://lequipe.fr),
  which is a 3.5.2 regression

BUG: 56109, 118277



 M  +16 -0     ChangeLog  
 M  +25 -0     html/html_baseimpl.cpp  
 M  +4 -0      html/html_baseimpl.h  
 M  +9 -5      khtmlview.cpp  
 M  +0 -5      rendering/render_form.h  
 M  +8 -0      rendering/render_frames.h  
 M  +7 -3      rendering/render_object.cpp  
 M  +5 -0      rendering/render_replaced.h  


--- branches/KDE/3.5/kdelibs/khtml/ChangeLog #555344:555345
@@ -1,3 +1,19 @@
+2006-06-27  Germain Garand  <germain@ebooksfrance.org>
+
+        . Correct drawing of borders on iframes and objects (#118277/#56109)
+        . Frameborder attribute for iframes (http://www.w3.org/TR/html401/present/frames.html#adef-frameborder) 
+        . Fix widget counter-mask not being updated in time, thus sometimes missing a repaint (e.g: http://lequipe.fr)
+
+        * html/html_baseimpl.{h,cpp} (HTMLIFrameElementImpl::parseAttribute): parse frameborder attribute. Defaults to true as per specification.
+        (HTMLIFrameElementImpl::updateFrame/attach): apply/remove frameborder style at attachment time.
+        * khtmlview.cpp (drawContents): fix counter-mask problem. Widget geometry is not accurate before painting, so we must 
+        use the RenderObject's.
+        * rendering/render_frames.h (paddingTop/paddingBottom/paddingLeft/paddingRight): reimplement. Frames have no padding.
+        (RenderPartObject::canHaveBorder): reimplement. True.
+        * rendering/render_object.cpp (RenderObject::updateWidgetMasks): clip mask to correct width/height, though it doesn't matter much on X11.
+        * rendering/render_replaced.h (RenderWidget::borderTop/borderBottom/borderLeft/borderRight): percolated down from RenderForm. 
+        Frames/Iframes also need that reimplementation.
+
 2006-06-22  Germain Garand  <germain@ebooksfrance.org>
 
         Implement floating auto-width table quirk
--- branches/KDE/3.5/kdelibs/khtml/html/html_baseimpl.cpp #555344:555345
@@ -584,6 +584,7 @@
     marginWidth = 0;
     marginHeight = 0;
     needWidgetUpdate = false;
+    m_frame = true;
 }
 
 HTMLIFrameElementImpl::~HTMLIFrameElementImpl()
@@ -615,17 +616,41 @@
         needWidgetUpdate = true; // ### do this for scrolling, margins etc?
         HTMLFrameElementImpl::parseAttribute( attr );
         break;
+    case ATTR_FRAMEBORDER:
+    {
+        m_frame = (!attr->val() || attr->value().toInt() > 0);
+        if (attached()) updateFrame();
+    }
     default:
         HTMLFrameElementImpl::parseAttribute( attr );
     }
 }
 
+void HTMLIFrameElementImpl::updateFrame()
+{
+    if (m_frame) {
+        addCSSProperty(CSS_PROP_BORDER_TOP_STYLE, CSS_VAL_OUTSET);
+        addCSSProperty(CSS_PROP_BORDER_BOTTOM_STYLE, CSS_VAL_OUTSET);
+        addCSSProperty(CSS_PROP_BORDER_LEFT_STYLE, CSS_VAL_OUTSET);
+        addCSSProperty(CSS_PROP_BORDER_RIGHT_STYLE, CSS_VAL_OUTSET);
+        addCSSLength(CSS_PROP_BORDER_WIDTH, "2");
+    } else {
+        addCSSProperty(CSS_PROP_BORDER_TOP_STYLE, CSS_VAL_NONE);
+        addCSSProperty(CSS_PROP_BORDER_BOTTOM_STYLE, CSS_VAL_NONE);
+        addCSSProperty(CSS_PROP_BORDER_LEFT_STYLE, CSS_VAL_NONE);
+        addCSSProperty(CSS_PROP_BORDER_RIGHT_STYLE, CSS_VAL_NONE);
+        removeCSSProperty(CSS_PROP_BORDER_WIDTH);
+    }
+
+}
+
 void HTMLIFrameElementImpl::attach()
 {
     assert(!attached());
     assert(!m_render);
     assert(parentNode());
 
+    updateFrame();
     name = getAttribute(ATTR_NAME);
     if (name.isNull())
         name = getAttribute(ATTR_ID);
--- branches/KDE/3.5/kdelibs/khtml/html/html_baseimpl.h #555344:555345
@@ -190,7 +190,11 @@
     virtual void recalcStyle( StyleChange ch );
 
 protected:
+
+    void updateFrame();
+
     bool needWidgetUpdate;
+    bool m_frame;
 };
 
 
--- branches/KDE/3.5/kdelibs/khtml/khtmlview.cpp #555344:555345
@@ -660,7 +660,14 @@
 	QWidget *w = it.current();
 	RenderWidget* rw = static_cast<RenderWidget*>( it.currentKey() );
 	if (w && rw && !rw->isKHTMLWidget()) { 
-            QRect g = w->geometry();
+            int x, y;
+            rw->absolutePosition(x, y);
+            contentsToViewport(x, y, x, y);
+            int pbx = rw->borderLeft()+rw->paddingLeft();
+            int pby = rw->borderTop()+rw->paddingTop();
+            QRect g = QRect(x+pbx, y+pby, 
+                            rw->width()-pbx-rw->borderRight()-rw->paddingRight(), 
+                            rw->height()-pby-rw->borderBottom()-rw->paddingBottom());
             if ( !rw->isFrame() && ((g.top() > pt.y()+eh) || (g.bottom() <= pt.y()) ||
                                     (g.right() <= pt.x()) || (g.left() > pt.x()+ew) ))
                 continue;
@@ -673,10 +680,7 @@
                 mask = mask.intersect( QRect(g.x(),g.y(),g.width(),g.height()) );
                 cr -= mask;
             } else {
-                int x, y;
-                rw->absolutePosition(x,y);
-                contentsToViewport(x,y,x,y);
-                cr -= QRect(x,y,rw->width(),rw->height());
+                cr -= g;
             }
         }
     }
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_form.h #555344:555345
@@ -78,11 +78,6 @@
 
     virtual bool isFormElement() const { return true; }
 
-    virtual int borderTop() const { return canHaveBorder() ? RenderWidget::borderTop() : 0; }
-    virtual int borderBottom() const { return canHaveBorder() ? RenderWidget::borderBottom() : 0; }
-    virtual int borderLeft() const { return canHaveBorder() ? RenderWidget::borderLeft() : 0; }
-    virtual int borderRight() const { return canHaveBorder() ? RenderWidget::borderRight() : 0; }
-
     // form elements never have padding
     virtual int paddingTop() const { return 0; }
     virtual int paddingBottom() const { return 0; }
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_frames.h #555344:555345
@@ -130,6 +130,12 @@
     virtual const char *renderName() const { return "RenderFrame"; }
     virtual bool isFrame() const { return true; }
 
+    // frames never have padding
+    virtual int paddingTop() const { return 0; }
+    virtual int paddingBottom() const { return 0; }
+    virtual int paddingLeft() const { return 0; }
+    virtual int paddingRight() const { return 0; }
+
     DOM::HTMLFrameElementImpl *element() const
     { return static_cast<DOM::HTMLFrameElementImpl*>(RenderObject::element()); }
 
@@ -150,6 +156,8 @@
 
     virtual void layout( );
     virtual void updateWidget();
+    
+    virtual bool canHaveBorder() const { return true; }
 
     virtual bool partLoadingErrorNotify( khtml::ChildFrame *childFrame, const KURL& url, const QString& serviceType );
 
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_object.cpp #555344:555345
@@ -2182,9 +2182,13 @@
             QRegion r = l ? l->getMask() : QRegion();
             int x,y;
             if (!r.isNull() && curr->absolutePosition(x,y)) {
-                x+= curr->borderLeft()+curr->paddingLeft();
-                y+= curr->borderBottom()+curr->paddingBottom();
-                r = r.intersect(QRect(x,y,curr->width(),curr->height()));
+                int pbx = curr->borderLeft()+curr->paddingLeft();
+                int pby = curr->borderTop()+curr->paddingTop();
+                x+= pbx;
+                y+= pby;
+                r = r.intersect(QRect(x,y,
+                                  curr->width()-pbx-curr->borderRight()-curr->paddingRight(),
+                                  curr->height()-pby-curr->borderBottom()-curr->paddingBottom()));
 #ifdef MASK_DEBUG
                 QMemArray<QRect> ar = r.rects();
                 kdDebug(6040) << "|| Setting widget mask for " << curr->information() << endl;
--- branches/KDE/3.5/kdelibs/khtml/rendering/render_replaced.h #555344:555345
@@ -146,6 +146,11 @@
     bool m_needsMask;
 
 public:
+    virtual int borderTop() const { return canHaveBorder() ? RenderReplaced::borderTop() : 0; }
+    virtual int borderBottom() const { return canHaveBorder() ? RenderReplaced::borderBottom() : 0; }
+    virtual int borderLeft() const { return canHaveBorder() ? RenderReplaced::borderLeft() : 0; }
+    virtual int borderRight() const { return canHaveBorder() ? RenderReplaced::borderRight() : 0; }
+
     class EventPropagator : public QWidget {
     public:
         void sendEvent(QEvent *e);