[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    branches/KDE/4.1/kdelibs/khtml
From:       Maks Orlovich <maksim () kde ! org>
Date:       2008-10-25 18:11:25
Message-ID: 1224958285.401411.601.nullmailer () svn ! kde ! org
[Download RAW message or body]

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<DOM::HTMLFormElementImpl&>(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<KHTMLPart*>(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;


[prev in list] [next in list] [prev in thread] [next in thread]