Views exposed text filter set to required shows an empty error and form error on page load

I can see that in Drupal 9.5.3 the patch is already included in core, but I still have see problem.

I'm using the most recent version of 9.5, and I still experience this issue.

I modified the previous patch to only exclude required exposed text filters from auto processing. I only excluded 'string' and 'combine'. I'm not sure if there are other text plugin ids that should be excluded as well, but these are two that I came across when I encountered this issue. I did verify this does not cause the regression outlined in #77.

The MR fixes the issue I was seeing.

Can MR be updated for 11.x as the latest development branch

Also will need test case showing the issue

I found this problem also occurred with a webform plugin we were using and decided hard coding the plugin id types was a bad idea. So I reworked the patch to just test for any input regardless of plugin type. I also moved the code outside of the reset button loop so that it works when the form does not contain a reset button. This does not cause the regression in #77 as it's looking at the form state for the input values instead of the view. I also added a test to demonstrate the problem, however, I don't have much experience with tests so if I need to change anything let me know. Opened a new merge request for 11.x, I don't think I have permissions to close the other one.

Seems the update caused a few tests to fail.

Haven't tested the new solution but it should be checked against media library as that's where the tests appear to be failing

Thanks for picking this up.

Ok, I added back the boolean plugin exclusion you had in there before. I don't quite follow why that value isn't in the form state, seems to be a behavior of the status or admin media filter, but that passes the tests anyway.

Reviewing MR 4560 for 11.x

Followed the steps from #77 for the regression that was found
With the MR applied I'm only seeing Article node

I think the test should be updated though to match that scenario, to show that the required filter runs initially.

Thanks and good work!

Ok, I have added the regression test as well.

Thanks for expanding the test coverage. Think this is ready for committer review.

I'm triaging RTBC issues → . I read the IS and the comments since the revert and didn't find any unanswered questions. It would help if the proposed resolution explained how the error message would not be shown. I found that in #88.

Thanks for working on this! I do enjoy helping to complete long standing issues.

I read the MR and tested locally. I make some comment in the MR about that. One of those is about testing and because of that I am setting this to NW.

I followed the steps to reproduce in #77 on HEAD and get this error, "The submitted value All in the type element is not allowed.". That error does go away when the MR is applied. However, a check should be made to see if there a duplicate issue about that error. I am not able to do that now.

I could not find an existing ticket about that error.

I added info from #88 to the IS

I ran into the issue in #95 while writing the tests and I don't think it's related to this issue. I think it should be opened as a separate issue as it happens when a filter is required but has the "All" option checked for checkbox options.

Thanks @solideogloria ! Restoring status.

Thanks for following up on #95.

From #95,

I make some comment in the MR about that. One of those is about testing and because of that I am setting this to NW.

Unfortunately, it seems that I did not save those comments. They are saved now. And this is now back to Needs work. Sorry about the delay.

Also, can someone explain which MR we should be using, and maybe hide the other one?

This also doesn't apply to Drupal 10.2, so Needs Reroll

I made sure that the problem in #3327193: Regression: Views with exposed filters which are required need the apply button pressed to show results → doesn't happen (I ensured that the required filter doesn't mean that Apply has to be clicked to see the results if the filter already has a value selected). That's the reason for the third test.

The reason for the tests should also be a bit more clear in the code's comments now.

With the fix removed these are the only two assertions in the test that fail. The assertions later in this test pass. I think that requires investigation to make sure the test is doing what it should be.

I understand that the third test passes without the code changes. However, this is just a test added to ensure that future changes to the code don't break this functionality, per #92 🐛 Views exposed text filter set to required shows an empty error and form error on page load Needs work .

@quietone The MR says there are still unresolved threads, but they were fixed in earlier commits on the MR. Can you mark them resolved? Thanks.

https://git.drupalcode.org/project/drupal/-/merge_requests/4560#note_251278

i am getting error after applying MR #102. This says that the file is no longer exists in drupal 11.x-dev

STR:
1. Goto admin/structure/views/view/content
2. Select Content: Title (exposed) from filter criteria.
3. Select Required.
4. Click on Apply.
5. Scroll Down.
6. Observe that the Error appears and the Title filed is outlined in red.
7. Apply the patch.

SS

It appears the file was renamed. It is now "test_exposed_form_required_filters.yml". You probably need to update your config or reset your dev environment. It's fine for me.

This actually pertains to code in the views module, not views_ui.

MR feedback appears to be addressed and does address the issue described in the issue summary.

drunken monkey → made their first commit to this issue’s fork.

I pushed a minor fix to comment phrasing (didn’t accurately describe the code) and code style (unnecessary check for !$form_values combined with empty()).

However, my actual problem with this MR goes much deeper: it seems to actually break the “Required” functionality of exposed filters in a major way, i.e., rendering it almost completely meaningless (except for adding the required="required" property, which browsers can use to implement client-side validation).
To test, I simply edited the pre-installed /admin/content view to make the “Title” filter required. With current HEAD it’s true that this leads to an empty error message that definitely shouldn’t be there. However, when applying the changes in MR !4560 I instead just got the normal view, listing all content, completely ignoring that the “Title” filter should have been required. (The only change was the little red asterisk next to “Title”.) When trying to submit the form without filling out “Title”, the browser (Mozilla Firefox 122.0) prevented me from doing that, but after manually removing the required attribute from the DOM this again worked as if the “Title” filter wasn’t required, just listing all content, as before, without complaint or error message.

I’m not exactly sure what this MR is trying to do, but it seems to me like in addition to the $form_state->setAlwaysProcess(FALSE); call we also need something that prevents the view from showing any results.

> I instead just got the normal view, listing all content, completely ignoring that the “Title” filter should have been required.

I think we need to define what is the expected behaviour when you load the page without a filter value by loading the URL rather than by submitting the form with the button. The problem is though, is it possible to tell the difference in PHP, since it's a GET form?

> When trying to submit the form without filling out “Title”, the browser (Mozilla Firefox 122.0) prevented me from doing that, but after manually removing the required attribute from the DOM this again worked as if the “Title” filter wasn’t required

I don't think this is a problem -- if someone edits their DOM then they're on their own.