Problem/Motivation

On #2664830: Add search capability to help topics, andypost/ambermatz discovered that if you go to admin/search/pages and disable the Help Search page, then any page (such as admin/help) that is displaying the Help Search block will have a WSOD.

The reason is that the Search Block Form doesn't verify that the page it is supposed to submit to exists and is active. It should.

Proposed resolution

Alternative 1:
A patch was proposed by jhodgdon on #2664830-203: Add search capability to help topics -- here's the interdiff:
https://www.drupal.org/files/issues/2019-10-08/interdiff_13.txt

It needs a test. Also the part for the doc block in the interface is on another issue. Only the SearchBlockForm needs the patch.

Alternative 2:
Another way to resolve this would be to remove the ability to disable search pages. It is not clear why you would want to disable them anyway.

Remaining tasks

1. Decide which way to go -- the consensus is that we should remove the ability to disable search pages.

2. Make a patch.

User interface changes

The ability to disable search pages will not exist any more. You will only be able to add and delete pages, not disable them.

API changes

Functions related to disabling search pages and checking status will be removed/deprecated.

Data model changes

The status component of search page plugin configuration will be removed.

Release notes snippet

TBD

CommentFileSizeAuthor
#26 interdiff_24-26.txt2.14 KBmeenakshi_j
#26 3086846-26.patch31.93 KBmeenakshi_j
#24 interdiff_22-24.txt832 bytesmeenakshi_j
#24 3086846-24.patch31.93 KBmeenakshi_j
#22 3086846-22.patch31.52 KBnikitagupta
#21 3086846-21.patch31.5 KBnikitagupta
#19 3086846-19.patch30.6 KBravi.shankar
#15 3086846-15.patch30.93 KBjhodgdon
#15 interdiff.txt19.89 KBjhodgdon
#13 3086846-13.patch14.07 KBjhodgdon

Comments

jhodgdon created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Looks here's a caching issue, because right after index disabled the block is shown but not search page available (I'm using standard profile)

andypost’s picture

andypost
he/him
Primary language Russian
commented

As I got working on #2901590-7: Investigate route rebuilding problem wrt Search module as that when search page enabled nothing calls route rebuild

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I think that the right solution here is that we should remove the ability to disable search pages. I think they should either be there or be deleted. Having disabled search pages is just an extra layer of complication that the Search module doesn't need.

Will ask for comments in Slack/DrupalChat.

pwolanin’s picture

pwolanin commented

+1 to removing the disable functionality and limiting to add/remove

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

It seems like (from the chat discussions and from the Search maintainer) we have a consensus to remove the ability to disable search pages. Updating summary.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: Disabling search page results in search block WSOD » Remove ability to disable search pages (no reason for it and causes problems and code complexity)

better title

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.9.x-dev » 9.0.x-dev

It will deprecate some usage in 9.x for 10.0 removal

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue tags: +Needs update path, +Needs update path tests

We will also need to have an update function. I suppose it will need to delete any SearchPage config items that have status => FALSE?

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think yes, we should delete them because otherwise enabling them could be more disruptive

Other thing is how to catch all usage of enable/disable which is scattered at all controllers...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Indeed. One way to find out what needs fixing might be to remove/deprecate the status fields/methods/properties/whatever from the plugin and config entity classes and schema, and see what breaks.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3086846-13.patch14.07 KB

This is interesting: The status flag is part of the config_entity base schema in core/config/schema/core.data_types.schema.yml, which the search page config schema inherits from. So I don't think we can get rid of it from the config schema. All we can do is ignore it, and remove it from the UI.

So. As a first pass at this, I've made a patch that overrides and deprecates the status methods for the search page config entity, and removes/fixes some other things that were testing/using status. Let's see what breaks!

We'll also still need an update and update test, which would remove any disabled search pages.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Weird automated test result. Oh, I think it's because 9.x wasn't working -- so I'll hit Retest.

Also one coding standards message to deal with once we see the test bot results:

/var/lib/drupalci/workspace/jenkins-drupal_patches-13416/source/core/modules/search/src/Entity/SearchPage.php
line 6	Unused use statement
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
StatusFileSize
new interdiff.txt19.89 KB
new 3086846-15.patch30.93 KB

Here's a second pass. Should fix a lot of the test fails.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Remove ability to disable search pages (no reason for it and causes problems and code complexity) » Deprecate ability to disable search pages (no reason for it and causes problems and code complexity)
Version: 9.0.x-dev » 9.1.x-dev
Status: Needs review » Needs work

In order to follow the continuous upgrade path, I think we need to deprecate the functionality in a minor release of D9. Then, it can be removed in 10.0.x. Thanks!

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3204343: Disabling the default search page can make the entire site inaccessible

As an alternative to this — here's a patch that just makes sure that SearchBlockForm never crashes: #3204343: Disabling the default search page can make the entire site inaccessible.

ravi.shankar’s picture

ravi.shankar commented
StatusFileSize
new 3086846-19.patch30.6 KB

Rerolled patch #15 on Drupal 9.2.x

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nikitagupta’s picture

nikitagupta commented
Status: Needs work » Needs review
StatusFileSize
new 3086846-21.patch31.5 KB

Rerolled patch #19 and fixed the failure.

nikitagupta’s picture

nikitagupta commented
StatusFileSize
new 3086846-22.patch31.52 KB

Rerolled patch #21

Status: Needs review » Needs work

The last submitted patch, 22: 3086846-22.patch, failed testing. View results

meenakshi_j’s picture

meenakshi_j commented
Status: Needs work » Needs review
StatusFileSize
new 3086846-24.patch31.93 KB
new interdiff_22-24.txt832 bytes
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work 
+++ b/core/modules/search/src/Controller/SearchController.php
@@ -201,19 +201,15 @@ public function editTitle(SearchPageInterface $search_page) {
+   * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Delete
...
+    trigger_error('SearchController::performOperation() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Delete unwanted search pages instead of disabling them.', E_USER_DEPRECATED);

+++ b/core/modules/search/src/Entity/SearchPage.php
@@ -261,4 +244,25 @@ protected function searchPluginManager() {
+   * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Delete
...
+    trigger_error('SearchPage::setStatus() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Delete unwanted search pages instead of disabling them.', E_USER_DEPRECATED);

should be 9.3.0

meenakshi_j’s picture

meenakshi_j commented
Status: Needs work » Needs review
StatusFileSize
new 3086846-26.patch31.93 KB
new interdiff_24-26.txt2.14 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/search/src/SearchPageListBuilder.php
@@ -320,9 +313,13 @@ public function getDefaultOperations(EntityInterface $entity) {
-    // Prevent the default search from being disabled or deleted.
+    // Do not allow enable/disable operations.
+    unset($operations['enable']);
+    unset($operations['disable']);

Interesting, I think this operations should not be there.

OTOH if contrib will add them then it should not be unset here

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

aaronbauman’s picture

aaronbauman
he/him
commented

Jumping in here with my use case, which I fear will be exacerbated by this change:
* Have a site with many nodes (1M+), only a small fraction of which I want to be searchable
* Install search_exclude module to create a search page meeting that criteria
* Default search is still enabled, so search tables in database still grow insanely large

How can I address this concern if the default search is forever enabled, and millions of nodes are getting indexed?

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

From what I can see #10 still needs to be addressed.

Did not review code.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Search Module was approved for removal in #3476883: [Policy, no patch] Move Search module to contrib .

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3565780: [meta] Tasks to deprecate the Search module and the removal work in #3565783: [meta] Tasks to remove the Search module.

Search will be moved to a contributed project before Drupal 12.0.0 is released.