SVN commit 875819 by orlovich: Fix how we find existing frames for the purpose of form target= submission. This fixes file upload for drupal 6/jquery.form --- its submit was incorrectly blocked as a popup attempt As we refactored this method, also use it for window.open, fixing us blocking opens to _self along the way. While testing this, I noticed that we can crash konq if we do window.open and one of the frames doesn't yet have a part yet. Added a workaround suggested by dfaure for now, as the proper fix is too risky and should be done on unstable branch only. BUG:172073 BUG:102044 M +15 -22 ecma/kjs_html.cpp M +21 -2 ecma/kjs_window.cpp M +2 -0 ecma/kjs_window.h M +3 -2 khtml_part.cpp --- branches/KDE/4.1/kdelibs/khtml/ecma/kjs_html.cpp #875818:875819 @@ -2074,42 +2074,35 @@ DOM::HTMLFormElementImpl& form = static_cast(element); if (id == KJS::HTMLElement::FormSubmit) { - KHTMLView *view = element.document()->view(); + KHTMLPart *part = element.document()->part(); KHTMLSettings::KJSWindowOpenPolicy policy = KHTMLSettings::KJSWindowOpenAllow; - if (view) - policy = view->part()->settings()->windowOpenPolicy(view->part()->url().host()); + if (part) + policy = part->settings()->windowOpenPolicy(part->url().host()); bool block = false; - if ( policy != KHTMLSettings::KJSWindowOpenAllow ) { block = true; - // if this is a form without a target, or a special target, don't block - QString trg = form.target().lower().string(); - if( trg.isEmpty() || trg == "_top" || trg == "_self" || - trg == "_parent") + // if this is a form without a target, don't block + if ( form.target().isEmpty() ) block = false; QString caption; // if there is a frame with the target name, don't block - if ( view && view->part() ) { - if (!view->part()->url().host().isEmpty()) - caption = view->part()->url().host() + " - "; - // search all (possibly nested) framesets - KHTMLPart *currentPart = view->part()->parentPart(); - while( currentPart != 0L ) { - if( currentPart->frameExists( form.target().string() ) ) - block = false; - currentPart = currentPart->parentPart(); - } + if ( part ) { + if (!part->url().host().isEmpty()) + caption = part->url().host() + " - "; + + if ( Window::targetIsExistingWindow(part, form.target().string()) ) + block = false; } - if ( block && policy == KHTMLSettings::KJSWindowOpenAsk && view ) { - if (view && view->part()) - emit view->part()->browserExtension()->requestFocus(view->part()); + if ( block && policy == KHTMLSettings::KJSWindowOpenAsk && part ) { + if (part ) + emit part->browserExtension()->requestFocus(part); caption += i18n( "Confirmation: JavaScript Popup" ); - if ( KMessageBox::questionYesNo(view, form.action().isEmpty() ? + if ( KMessageBox::questionYesNo(part->view(), form.action().isEmpty() ? i18n( "This site is submitting a form which will open up a new browser " "window via JavaScript.\n" "Do you want to allow the form to be submitted?" ) : --- branches/KDE/4.1/kdelibs/khtml/ecma/kjs_window.cpp #875818:875819 @@ -1546,6 +1546,20 @@ emit ext->moveTopLevelWidget( tl->x() + moveByX , tl->y() + moveByY ); } +bool Window::targetIsExistingWindow(KHTMLPart* ourPart, const QString& frameName) +{ + QString normalized = frameName.toLower(); + if (normalized == "_top" || normalized == "_self" || normalized == "_parent") + return true; + + // Find the highest parent part we can access. + KHTMLPart* p = ourPart; + while (p->parentPart() && p->parentPart()->checkFrameAccess(ourPart)) + p = p->parentPart(); + + return p->findFrame(frameName); +} + JSValue *Window::openWindow(ExecState *exec, const List& args) { KHTMLPart *part = qobject_cast(m_frame->m_part); @@ -1571,6 +1585,13 @@ KHTMLSettings::KJSWindowOpenPolicy policy = part->settings()->windowOpenPolicy(part->url().host()); + + QString frameName = args.size() > 1 ? args[1]->toString(exec).qstring() : QString("_blank"); + + // Always permit opening in an exist frame (including _self, etc.) + if ( targetIsExistingWindow( part, frameName ) ) + policy = KHTMLSettings::KJSWindowOpenAllow; + if ( policy == KHTMLSettings::KJSWindowOpenAsk ) { emit part->browserExtension()->requestFocus(part); QString caption; @@ -1593,8 +1614,6 @@ policy = KHTMLSettings::KJSWindowOpenAllow; } - QString frameName = args.size() > 1 ? args[1]->toString(exec).qstring() : QString("_blank"); - v = args[2]; QString features; if (v && v->type() != UndefinedType && v->toString(exec).size() > 0) { --- branches/KDE/4.1/kdelibs/khtml/ecma/kjs_window.h #875818:875819 @@ -114,6 +114,8 @@ void delayedGoHistory(int steps); void goHistory(int steps); void goURL(ExecState* exec, const QString& url, bool lockHistory); + + static bool targetIsExistingWindow(KHTMLPart* part, const QString& frameName); JSValue* openWindow(ExecState *exec, const List &args); JSValue* executeOpenWindow(ExecState *exec, const KUrl& url, const QString& frameName, const QString& features); void resizeTo(QWidget* tl, int width, int height); --- branches/KDE/4.1/kdelibs/khtml/khtml_part.cpp #875818:875819 @@ -6128,7 +6128,7 @@ ConstFrameIt it = d->m_frames.begin(); const ConstFrameIt end = d->m_frames.end(); for (; it != end; ++it ) - if (!(*it)->m_bPreloaded) + if (!(*it)->m_bPreloaded && (*it)->m_part) res += (*it)->m_name; return res; @@ -6141,7 +6141,8 @@ ConstFrameIt it = d->m_frames.begin(); const ConstFrameIt end = d->m_frames.end(); for (; it != end; ++it ) - if (!(*it)->m_bPreloaded) + if (!(*it)->m_bPreloaded && (*it)->m_part) // ### TODO: make sure that we always create an empty + // KHTMLPart for frames so this never happens. res.append( (*it)->m_part ); return res;