KeywordVocabulary override is not plone.volto specific

[szakitibi]

The KeywordsVocabulary is always replaced by the volto version, even if plone.volto is not installed for the Plone site and it runs on classic UI.

plone.volto/src/plone/volto/overrides.zcml

Lines 6 to 9 in fdb8923

	<utility
	name="plone.app.vocabularies.Keywords"
	component=".vocabularies.subject.KeywordsVocabularyFactory"
	/>

The override has been introduced with commit 1b96c31

It should be using zcml:condition="installed plone.volto".

[mauritsvanrees]

I don't know why there are two different vocabularies.

But zcml:condition="installed plone.volto" would not work. The "installed" check simply means: "is the plone.volto package available in the current Python process". This check runs at startup time, and cannot check whether plone.volto is activated in the Plone site.

[szakitibi]

But zcml:condition="installed plone.volto" would not work. The "installed" check simply means: "is the plone.volto package available in the current Python process". This check runs at startup time, and cannot check whether plone.volto is activated in the Plone site.

Yes, right, zcml:condition is not an option.

I don't know why there are two different vocabularies.

I have compared Plone 6.0.11 buildout's KeywordsVocabulary from plone.app.vocabulary version 5.0.5 with the KeywordsVocabulary from plone.volto version 4.4.0. There is not much to see, other then that plone.volto version seems to be a bit outdated.

             return None
         if registry.get("plone.subjects_of_navigation_root", False):
             portal = getToolByName(context, "portal_url").getPortalObject()
-            return get_navigation_root_object(context, portal)
+            return getNavigationRootObject(context, portal)
         return None
 
     def all_keywords(self, kwfilter):
...
         result = []
         # query all oids of path - low level
         pquery = {
-            self.path_index: {
-                "query": "/".join(section.getPhysicalPath()),
-                "depth": -1,
-            }
+            self.path_index: {"query": "/".join(section.getPhysicalPath()), "depth": -1}
         }
-        kwfilter = safe_text(kwfilter)
+        kwfilter = safe_encode(kwfilter)
         # uses internal zcatalog specific details to quickly get the values.
         path_result, info = path_idx._apply_index(pquery)
         for tag in tags_idx.uniqueValues():
-            if kwfilter and kwfilter not in safe_text(tag):
+            if kwfilter and kwfilter not in safe_encode(tag):
                 continue
             tquery = {self.keyword_index: tag}
             tags_result, info = tags_idx._apply_index(tquery)

The diff is the same for latest versions plone.app.vocabularies 6.0.0 compared to plone.volto 4.4.2

The override doc string states that the reason for the override is to "Override Keywords vocabulary to provide the real Keyword as the token.", but I also compared the original override committed on Jan 29, 2020 with plone.app.vocabularies 4.1.1 based on release history, and there is not much to see:

     # Allow users to customize the index to easily create
     # KeywordVocabularies for other keyword indexes
-    keyword_index = 'Subject'
-    path_index = 'path'
+    keyword_index = "Subject"
+    path_index = "path"
 
     def section(self, context):
         """gets section from which subjects are used.
...
         registry = queryUtility(IRegistry)
         if registry is None:
             return None
-        if registry.get('plone.subjects_of_navigation_root', False):
-            portal = getToolByName(context, 'portal_url').getPortalObject()
+        if registry.get("plone.subjects_of_navigation_root", False):
+            portal = getToolByName(context, "portal_url").getPortalObject()
             return getNavigationRootObject(context, portal)
         return None
 
     def all_keywords(self, kwfilter):
         site = getSite()
-        self.catalog = getToolByName(site, 'portal_catalog', None)
+        self.catalog = getToolByName(site, "portal_catalog", None)
         if self.catalog is None:
             return SimpleVocabulary([])
         index = self.catalog._catalog.getIndex(self.keyword_index)
...
     def keywords_of_section(self, section, kwfilter):
         """Valid keywords under the given section.
         """
-        pcat = getToolByName(section, 'portal_catalog')
+        pcat = getToolByName(section, "portal_catalog")
         cat = pcat._catalog
         path_idx = cat.indexes[self.path_index]
         tags_idx = cat.indexes[self.keyword_index]
         result = []
         # query all oids of path - low level
         pquery = {
-            self.path_index: {
-                'query': '/'.join(section.getPhysicalPath()),
-                'depth': -1,
-            }
+            self.path_index: {"query": "/".join(section.getPhysicalPath()), "depth": -1}
         }
         kwfilter = safe_encode(kwfilter)
         # uses internal zcatalog specific details to quickly get the values.

Seems like, the actual point is to patch how the terms are generated
and to replace the safe_simplevocabulary_from_values.

The original issue plone/plone.restapi#782 states: "This seems to work in the legacy plone interface too". This is probably true, since plone.volto is now part of Plone 6, and Plone 6 Classic UI runs on this override version of KeywordsVocabulary for years now.

As far as I see there is no point overriding the vocabulary. It should either patch the safe_simplevocabulary_from_values and make a plone.volto.interfaces.IPloneVoltoCoreLayer check, or alternatively the whole thing could be moved into plone.app.vocabularies, since it is basically replacing the KeywordsVocabulary for Plone 6 anyways.