[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#305481: Patch for knode brokenly forcing followups



I submitted the attached patch to fix this but to KDE bugzilla last night
https://bugs.kde.org/show_bug.cgi?id=68732

Summarising for debbug:
1. Followup-To was being populated from Newsgroups at start and every
time Newsgroups changed;
2. Followup-To widget was a single entry combo box with no default set, so
would always set followups to go to the first entry in Newsgroups.

Fix:
1. Don't automatically set Followup-To
2. Change Followup-To widget into a text box to permit user free form
editing without defaulting to any single group.  This is no more lax than
previously, as user could type in an free form entry into the combo box.

This patch is still not fully GNKSA compliant, but is slightly more
so than before due to not forcing incorrect followups, and is far more
usable than the combo box.

Nick
--- kncomposer.h.orig	2006-02-02 12:35:43.000000000 +0000
+++ kncomposer.h	2006-02-02 21:59:16.000000000 +0000
@@ -228,7 +228,7 @@
     KNLineEdit   *g_roups;
     KNLineEdit  *t_o;
 
-    KComboBox   *f_up2;
+    KNLineEdit  *f_up2;
     QPushButton *g_roupsBtn,
                 *t_oBtn;
 
--- kncomposer.cpp.orig	2005-09-10 09:24:04.000000000 +0100
+++ kncomposer.cpp	2006-02-02 22:00:00.000000000 +0000
@@ -569,8 +569,8 @@
     }
 
     int groupCount = QStringList::split(',',v_iew->g_roups->text()).count();
-    int fupCount = QStringList::split(',',v_iew->f_up2->currentText()).count();
-    bool followUp = !v_iew->f_up2->currentText().isEmpty();
+    int fupCount = QStringList::split(',',v_iew->f_up2->text()).count();
+    bool followUp = !v_iew->f_up2->text().isEmpty();
 
     if (groupCount>12) {
       KMessageBox::sorry(this, i18n("You are crossposting to more than 12 newsgroups.\nPlease remove all newsgroups in which your article is off-topic."));
@@ -759,8 +759,8 @@
     a_rticle->setDoMail(false);
 
   //Followup-To
-  if( a_rticle->doPost() && !v_iew->f_up2->currentText().isEmpty())
-    a_rticle->followUpTo()->fromUnicodeString(v_iew->f_up2->currentText(), KMime::Headers::Latin1);
+  if( a_rticle->doPost() && !v_iew->f_up2->text().isEmpty())
+    a_rticle->followUpTo()->fromUnicodeString(v_iew->f_up2->text(), KMime::Headers::Latin1);
   else
     a_rticle->removeHeader("Followup-To");
 
@@ -898,6 +898,7 @@
 
 void KNComposer::initData(const QString &text)
 {
+  kdDebug(5003) << "KNComposer::initData" << endl;
   //Subject
   if(a_rticle->subject()->isEmpty())
     slotSubjectChanged(QString::null);
@@ -912,8 +913,10 @@
 
   //Followup-To
   KMime::Headers::FollowUpTo *fup2=a_rticle->followUpTo(false);
+  kdDebug(5003) << "fup2 init as " << fup2 << endl;
   if(fup2 && !fup2->isEmpty())
-    v_iew->f_up2->lineEdit()->setText(fup2->asUnicodeString());
+    v_iew->f_up2->setText(fup2->asUnicodeString());
+  kdDebug(5003) << "fup2 now " << fup2 << endl;
 
   KMime::Content *textContent=a_rticle->textContent();
   QString s;
@@ -1435,25 +1438,37 @@
     setCaption( i18n("No Subject") );
 }
 
+// TODO for GNKSA: Write a function to check that Followup-To is a strict
+// subset of Newsgroups.  Use that function before posting.  Don't call
+// it while our user is still flipping between fields as it will only
+// annoy her.  If followups are identical to newsgroups when posting,
+// then remove them as they're redundant.
+// TODO (gold plated): make Followup-To into a clever custom widget which
+// normally looks like a text box so the user can see the full entry
+// (i.e. is NOT a single entry combo or other cumbersome drop-down). This
+// widget will let user select multiple entries from Newsgroups, whilst
+// making sure not to set anything unless the user explicitly asks us to.
 
 void KNComposer::slotGroupsChanged(const QString &t)
 {
-  KQCStringSplitter split;
-  bool splitOk;
-  QString currText=v_iew->f_up2->currentText();
-
-  v_iew->f_up2->clear();
-
-  split.init(t.latin1(), ",");
-  splitOk=split.first();
-  while(splitOk) {
-    v_iew->f_up2->insertItem(QString::fromLatin1(split.string()));
-    splitOk=split.next();
-  }
-  v_iew->f_up2->insertItem("");
+  // Leaving this call in, as it may be re-usable for the gold plated TODO.
+
+  // KQCStringSplitter split;
+  // bool splitOk;
+  // QString currText=v_iew->f_up2->currentText();
+
+  // v_iew->f_up2->clear();
+
+  // split.init(t.latin1(), ",");
+  // splitOk=split.first();
+  // while(splitOk) {
+    // v_iew->f_up2->insertItem(QString::fromLatin1(split.string()));
+    // splitOk=split.next();
+  // }
+  // v_iew->f_up2->insertItem("");
 
-  if ( !currText.isEmpty() || !mFirstEdit ) // user might have cleared fup2 intentionally during last edit
-    v_iew->f_up2->lineEdit()->setText(currText);
+  // if ( !currText.isEmpty() || !mFirstEdit ) // user might have cleared fup2 intentionally during last edit
+    // v_iew->f_up2->lineEdit()->setText(currText);
 }
 
 
@@ -1760,7 +1775,9 @@
   connect(g_roupsBtn, SIGNAL(clicked()), parent(), SLOT(slotGroupsBtnClicked()));
 
   //Followup-To
-  f_up2=new KComboBox(true, hdrFrame);
+  f_up2=new KNLineEdit(this, false, hdrFrame);
+  mEdtList.append(f_up2);
+
   l_fup2=new QLabel(f_up2, i18n("Follo&wup-To:"), hdrFrame);
   hdrL->addWidget(l_fup2, 2,0);
   hdrL->addMultiCellWidget(f_up2, 2,2, 1,2);

Reply to: