SVN commit 825688 by orlovich: Fix a bunch of bugs in iframe notification that showed up on hosted SunSpider: 1) Be careful to only create a name for frames once, so we can associate its ChildFrame with it, and know when it's completed, etc. 2) Always let requestFrame (via computeContent) handle the frame navigation since that handles mimetype changes properly 3) Be careful to not destroy the widget (and hence the part) improperly. M +19 -25 html_baseimpl.cpp M +1 -1 html_baseimpl.h M +3 -0 html_objectimpl.cpp --- trunk/KDE/kdelibs/khtml/html/html_baseimpl.cpp #825687:825688 @@ -259,20 +259,20 @@ void HTMLFrameElementImpl::ensureUniqueName() { - // Grab initial name if need be.. - if (name.isEmpty()) { - name = getAttribute(ATTR_NAME); - if (name.isNull()) - name = getAttribute(ATTR_ID); - } + // If we already have a name, don't do anything. + if (!name.isEmpty()) + return; - // Ensure uniqueness... + // Use the specified name first.. + name = getAttribute(ATTR_NAME); + if (name.isNull()) + name = getAttribute(ATTR_ID); + + // Generate synthetic name if there isn't a natural one or + // if the natural one conflicts KHTMLPart* parentPart = document()->part(); - if (parentPart) { - // we need a unique name for every frame in the frameset. Hope that's unique enough. - if (name.isEmpty() || parentPart->frameExists( name.string() ) ) - name = DOMString(parentPart->requestFrameName()); - } + if (name.isEmpty() || parentPart->frameExists( name.string() ) ) + name = DOMString(parentPart->requestFrameName()); } void HTMLFrameElementImpl::parseAttribute(AttributeImpl *attr) @@ -318,12 +318,6 @@ setHTMLEventListener(EventImpl::UNLOAD_EVENT, document()->createHTMLEventListener(attr->value().string(), "onunload", this)); break; - case ATTR_ID: - case ATTR_NAME: - // FIXME: if already attached, doesn't change the frame name - // FIXME: frame name conflicts, no unique frame name anymore - name = attr->value(); - //fallthrough intentional, let the base handle it default: HTMLElementImpl::parseAttribute(attr); } @@ -381,8 +375,6 @@ if (!parentPart) return; - ensureUniqueName(); - // Bail out on any disallowed URLs if( !document()->isURLAllowed(url.string()) ) return; @@ -395,9 +387,10 @@ return; } - // Go ahead and load a part... - // ### double-check how multiple requests are handled in the part! - clearChildWidget(); + ensureUniqueName(); + + // Go ahead and load a part... We don't need to clear the widget here, + // since the -frames- have their lifetime managed, using the name uniqueness. parentPart->requestFrame( this, url.string(), name.string() ); } @@ -731,8 +724,8 @@ addHTMLAlignment( attr->value() ); break; case ATTR_SRC: + url = khtml::parseURL(attr->val()); setNeedComputeContent(); - HTMLFrameElementImpl::parseAttribute( attr ); break; case ATTR_FRAMEBORDER: { @@ -797,7 +790,8 @@ return; ensureUniqueName(); - clearChildWidget(); + // Since we have a name, we can request even if we just want to navigate, + // since KHTMLPart will make sure to reuse the KPart if the mimetype doesn't change parentPart->requestFrame(this, url.string(), name.string(), QStringList(), true); } --- trunk/KDE/kdelibs/khtml/html/html_baseimpl.h #825687:825688 @@ -108,7 +108,7 @@ KHTMLPart* contentPart() const; DOMString url; - DOMString name; + DOMString name; //Computed name for the frame map int marginWidth; int marginHeight; --- trunk/KDE/kdelibs/khtml/html/html_objectimpl.cpp #825687:825688 @@ -99,6 +99,9 @@ void HTMLPartContainerElementImpl::setWidget(QWidget* widget) { + if (widget == m_childWidget) + return; // The same part got navigated. Don't do anything + QWidget* oldWidget = m_childWidget; m_childWidget = widget; if (m_childWidget)