Bug 22680 - Can't add "increase/decrease font sizes" icons to location toolbar
Summary: Can't add "increase/decrease font sizes" icons to location toolbar
Status: RESOLVED WORKSFORME
Alias: None
Product: kdelibs
Classification: Frameworks and Libraries
Component: kedittoolbar (show other bugs)
Version: unspecified
Platform: RedHat Enterprise Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
: 51444 58053 58871 60854 (view as bug list)
Depends on:
Blocks:
 
Reported: 2001-03-19 07:18 UTC by Nathaniel Gray
Modified: 2008-04-27 08:57 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nathaniel Gray 2001-03-19 07:16:28 UTC
(*** This bug was imported into bugs.kde.org ***)

Package:           konqueror
Version:           KDE 2.1.0 
Severity:          normal
Installed from:    RedHat RPMs
Compiler:          Not Specified
OS:                Linux
OS/Compiler notes: Stock RedHat 6.2 compiler egcs (I forget which exact version)

Just like the title says.  I can add other icons but the "Increase/Decrease Font Sizes" ones just don't show up.  I am able to move them to the "Current Actions" (Current tools would be a less confusing name by the way) list in the Configure Toolbar dialog but they don't appear on the actual toolbar.

These icons do show up in the main toolbar but not the location toolbar.

(Submitted via bugs.kde.org)
Comment 1 Maksim Orlovich 2002-05-05 04:56:03 UTC
Can still reproduce it with KDE: 3.0.5 (CVS HEAD >= 20020427)
Comment 2 John Firebaugh 2002-07-24 03:13:18 UTC
The problem is that Increase/Decrease Font Size are KHTMLPart actions.=20
KHTMLPart has only a main toolbar entry in its ui file. In KEditToolbar yo=
u=20
can add any part's actions to any toolbar. Then the gui entries might be=
=20
written out to the wrong local ui file (konquerorui.rc instead of=20
khtml_browser.rc in this case).

I can think of several ways to fix this.
0) Disallow inserting part actions in shell toolbars via KEditToolbar.
1) Add an empty location toolbar node to khtmlpart.ui.
2) Have KEditToolbar display an artificial "Location Toolbar <khtmlpart>"=
=20
entry in the dropdown.
3) Implement proper merging of toolbar actions in KEditToolbar. I'd really=
=20
like to see this happen but it would probably be quite difficult to get=20
right.

Anyone got any other ideas?
Comment 3 David Faure 2002-07-25 13:50:24 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 24 July 2002 05:13 John Firebaugh wrote:
> The problem is that Increase/Decrease Font Size are KHTMLPart actions. 
> KHTMLPart has only a main toolbar entry in its ui file. In KEditToolbar you 
> can add any part's actions to any toolbar. Then the gui entries might be 
> written out to the wrong local ui file (konquerorui.rc instead of 
> khtml_browser.rc in this case).

Ouch.

> I can think of several ways to fix this.
> 0) Disallow inserting part actions in shell toolbars via KEditToolbar.
Maybe the only possible short term fix... not nice though.

> 1) Add an empty location toolbar node to khtmlpart.ui.
Doesn't fix the general case.

> 2) Have KEditToolbar display an artificial "Location Toolbar <khtmlpart>" 
> entry in the dropdown.
It still requires the user to pick up the right item. This goes the wrong way.
If it only works when saving in khtmlpart then we simply want to show
"Location Toolbar" and let the destination xml file depend on the action
to be saved. That sounds like what you called 3).

> 3) Implement proper merging of toolbar actions in KEditToolbar. I'd really 
> like to see this happen but it would probably be quite difficult to get 
> right.

It sounds like the correct fix though. I defended "A <b>" by saying
"otherwise we don't know in which XMLGUI file to add the action"
but you're saying: we DO know which file to use it's the one used by
the XMLGUIClient that provides this action.

- -- 
David FAURE david@mandrakesoft.com faure@kde.org
http://people.mandrakesoft.com/~david/
Contributing to: http://www.konqueror.org/ http://www.koffice.org/
Back from holidays - 1750 mails
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE9QAIg72KcVAmwbhARAlURAKCjSe36kh7kvQ6E3yhs6MMpkxdGCACfV1jm
3wv78LFkGlRkU7CyTql0Xs4=
=8F97
-----END PGP SIGNATURE-----
Comment 4 Tobias Koenig 2003-11-01 18:36:08 UTC
*** Bug 58053 has been marked as a duplicate of this bug. ***
Comment 5 Tobias Koenig 2003-11-01 18:50:16 UTC
*** Bug 51444 has been marked as a duplicate of this bug. ***
Comment 6 Tobias Koenig 2003-11-01 19:27:00 UTC
*** Bug 58871 has been marked as a duplicate of this bug. ***
Comment 7 Tobias Koenig 2003-11-01 21:14:26 UTC
*** Bug 60854 has been marked as a duplicate of this bug. ***
Comment 8 David Faure 2004-04-28 18:26:44 UTC
Full details of what needs to be done are in kdelibs/kdeui/TODO.xmlgui
Not an afternoon hack...
Comment 9 David Faure 2004-04-28 20:08:38 UTC
CVS commit by faure: 

Most often reported kedittoolbar bug: adding some actions to some toolbars doesn't work.
This is because the xmlgui only looks at the actions from each client's actioncollection.
So the easiest way to handle this is to only show the actions for the selected client.

This commit also fixes my "giving icons to actions" code by saving the icon in the right
file only (instead of all of them!)

CCMAIL: 72095-done@bugs.kde.org, 28397@bugs.kde.org, 22680@bugs.kde.org, 52242-done@bugs.kde.org, 75676-done@bugs.kde.org


  M +26 -26    kedittoolbar.cpp   1.88


--- kdelibs/kdeui/kedittoolbar.cpp  #1.87:1.88
@@ -68,4 +68,5 @@ public:
   {
     m_isModified = false;
+    m_actionCollection = 0;
   }
 
@@ -74,4 +75,5 @@ public:
   XmlType      m_type;
   bool         m_isModified;
+  KActionCollection* m_actionCollection;
 
   ToolbarList  m_barList;
@@ -190,5 +192,5 @@ public:
   }
 
-  QValueList<KAction*> m_actionList;
+  //QValueList<KAction*> m_actionList;
   KActionCollection* m_collection;
   KInstance         *m_instance;
@@ -359,7 +361,5 @@ void KEditToolbarWidget::initNonKPart(KA
                                       const QString& file, bool global)
 {
-  // let's not forget the stuff that's not xml specific
-  //d->m_collection = *collection;
-  d->m_actionList = collection->actions();
+  //d->m_actionList = collection->actions();
 
   // handle the merging
@@ -380,7 +380,8 @@ void KEditToolbarWidget::initNonKPart(KA
   KXMLGUIFactory::removeDOMComments( elem );
   local.m_barList = d->findToolbars(elem);
+  local.m_actionCollection = collection;
   d->m_xmlFiles.append(local);
 
-  // then, the merged one
+  // then, the merged one (ui_standards + local xml)
   XmlData merge;
   merge.m_xmlFile  = QString::null;
@@ -389,4 +390,5 @@ void KEditToolbarWidget::initNonKPart(KA
   elem = merge.m_document.documentElement().toElement();
   merge.m_barList  = d->findToolbars(elem);
+  merge.m_actionCollection = collection;
   d->m_xmlFiles.append(merge);
 
@@ -423,7 +425,8 @@ void KEditToolbarWidget::initKPart(KXMLG
     KXMLGUIFactory::removeDOMComments( elem );
     data.m_barList = d->findToolbars(elem);
+    data.m_actionCollection = client->actionCollection();
     d->m_xmlFiles.append(data);
 
-    d->m_actionList += client->actionCollection()->actions();
+    //d->m_actionList += client->actionCollection()->actions();
   }
 
@@ -700,4 +703,7 @@ void KEditToolbarWidget::loadActionList(
   m_downAction->setEnabled(false);
 
+  // We'll use this action collection
+  KActionCollection* actionCollection = d->m_currentXmlData.m_actionCollection;
+
   // store the names of our active actions
   QMap<QString, bool> active_list;
@@ -738,8 +744,10 @@ void KEditToolbarWidget::loadActionList(
     }
 
-    // iterate through all of our actions
-    for (unsigned int i = 0;  i < d->m_actionList.count(); i++)
+    // iterate through this client's actions
+    // This used to iterate through _all_ actions, but we don't support
+    // putting any action into any client...
+    for (unsigned int i = 0;  i < actionCollection->count(); i++)
     {
-      KAction *action = d->m_actionList[i];
+      KAction *action = actionCollection->action( i );
 
       // do we have a match?
@@ -762,7 +770,7 @@ void KEditToolbarWidget::loadActionList(
 
   // go through the rest of the collection
-  for (int i = d->m_actionList.count() - 1; i > -1; --i)
+  for (int i = actionCollection->count() - 1; i > -1; --i)
   {
-    KAction *action = d->m_actionList[i];
+    KAction *action = actionCollection->action( i );
 
     // skip our active ones
@@ -1134,20 +1142,12 @@ void KEditToolbarWidget::slotChangeIcon(
   item->setPixmap(0, BarIcon(icon, 16));
 
-  // Very much like the beginning of updateLocal
-  XmlDataList::Iterator xit = d->m_xmlFiles.begin();
-  for ( ; xit != d->m_xmlFiles.end(); ++xit)
-  {
-    if ( (*xit).m_type == XmlData::Merged )
-      continue;
-
-    (*xit).m_isModified = true;
+  d->m_currentXmlData.m_isModified = true;
 
     // Get hold of ActionProperties tag
-    QDomElement elem = KXMLGUIFactory::actionPropertiesElement( (*xit).m_document );
+  QDomElement elem = KXMLGUIFactory::actionPropertiesElement( d->m_currentXmlData.m_document );
     // Find or create an element for this action
     QDomElement act_elem = KXMLGUIFactory::findActionByName( elem, item->internalName(), true /*create*/ );
     Q_ASSERT( !act_elem.isNull() );
     act_elem.setAttribute( "icon", icon );
-  }
 }
 


Comment 10 Nicolas L. 2005-05-15 02:01:32 UTC
this bug is close with this commit ? if yes we could close this bug :)
Comment 11 George Goldberg 2008-02-15 05:30:36 UTC
Does the commit in comment #9 fix this bug? If so, please close it.
Comment 12 Derick Swanepoel 2008-02-18 10:52:43 UTC
I think it does. One is no longer able to add Enlarge/Shrink Font to Current Actions in the dialog.
Comment 13 George Goldberg 2008-04-27 08:57:21 UTC
Seems to be fixed in trunk. Please reopen if it can be reproduced.