- 🇨🇦Canada tame4tex
I have been able to replicate this bug on version 4.3.1.
In our instance, we had custom functionality that added a hidden element to the Views Exposed Form that was unique for each new form build.
Here is the workflow of the cause of the bug:
- When the VBO form was submitted again after returning via the browser back button, the exposed form input had changed, triggering
$this->tempStoreData['list']
to be cleared inViewsBulkOperationsBulkForm::updateTempstoreData()
. - Then in
ViewsBulkOperationsBulkForm::viewsFormSubmit()
because the selected key was not in$this->tempStoreData['bulk_form_keys']
nothing got re-added to$this->tempStoreData['list']
. - Then finally in
ViewsBulkOperationsActionProcessor::executeProcessing()
because$data['list']
is empty it gets set to all view results and ALL view results get processed!!
If we removed the custom hidden element, the bug didn't occur.
Even though it is a specific set of circumstances that would trigger the bug, I feel it is a critical issue given that data loss (e.g. with delete node action) or security issues (e.g. with unblock users action) could occur.
I have been able to create a FunctionalJavascript test locally but it requires the browser to be in non headless mode. Not sure if it will work with Gitlab CI but I will add a MR for this test shortly.
- When the VBO form was submitted again after returning via the browser back button, the exposed form input had changed, triggering
- 🇨🇦Canada tame4tex
So it took some fiddling with the chrome options to get back/forward cache working but the test is now failing as expected.
https://git.drupalcode.org/issue/views_bulk_operations-3199534/-/pipelin...
I will take a look at the patch from #2 tomorrow and create a MR with a potential fix.
- Merge request !91Issue #3199534: Add validation and testing to prevent issues from browser back button bug → (Open) created by tame4tex
- 🇨🇦Canada tame4tex
I have added MR !91. This was inspired by
on_configure_form_every_entity_is_selected-3199534-02.patch
and although it doesn't specifically fix the bug, it prevents the bug from causing unexpected data changes by throwing a validation error when the bug conditions occur.The problem with patch #2, apart from needing to be modified to apply to updated code, is it just displays the error message
No items selected.
. After getting the error, if the user continuously selects options without reloading the page, the same error message will keep appearing. So I updated the error message to provide a link to reload the page , along with helpful info to prevent the problem form occurring in the first place.Ideally the code needs to be modified to correctly handle back/forward cache, but I believe that is best served in another issue as it is more complex and will most likely need greater input from @graber.
- 🇨🇦Canada tame4tex
Updated the MR to add a a pageshow event listener which will prevent the bug from occurring in the first place. As suggested on https://web.dev/articles/bfcache.
I still think it is worth the extra measure to keep the validation and sanity check in place "just in case".
- Status changed to Needs review
4 months ago 1:17pm 28 March 2025 - First commit to issue fork.
- 🇮🇳India hemant_raghuvanshi
I have encountered the same issue while using VBO version 4.3.4. I also reviewed Merge Request !91 and Comment #9, and created a patch based on the following diff:
https://git.drupalcode.org/project/views_bulk_operations/-/merge_request...
However, the issue still persists at the code level.In the file:
views_bulk_operations/src/Plugin/views/field/ViewsBulkOperationsBulkForm.php
There is a condition:
// Update and fetch tempstore data to be available from this point
// as it's needed for proper functioning of further logic.
// Update tempstore data with bulk form keys only when the form is
// displayed, but not when the form is being built before submission
// (data is subject to change - new entities added or deleted after
// the form display). TODO: consider using $form_state->set() instead.
if (empty($form_state->getUserInput()['op'])) {
$this->updateTempstoreData($entity_data);
}
else {
$this->updateTempstoreData();
}After clicking the browser's back button, the flow always goes into the else block, resulting in empty data.
If we assign $entity_data in the else block as well, the functionality works as expected. - 🇨🇦Canada tame4tex
Updated MR to fix bug in previous commit which was causing test
testViewsBulkOperationsWithDynamicInsertion
to fail.Also removed the
views_bulk_operations.libraries.yml
version
declaration. As outlined on https://www.drupal.org/docs/develop/creating-modules/adding-assets-css-j... →Starting from Drupal 10.1.2, the version information within a library definition plays a critical role in generating a unique hash for aggregated files. Consequently, it is imperative that the "version" in a library definition is updated whenever a referenced CSS/JS file undergoes changes.
Alternatively, if the version is omitted, the prior behavior will apply, where the content of referenced CSS/JS files is utilized in the hash.
Incorrect usage of version information could lead to browser and edge cache invalidation issues.
The update of this version was missed in this MR and also in commit 6234592e. So rather than risk it being missed again, I thought it best to remove altogether.
@hemant_raghuvanshi:
Could you please try the latest version of the MR and let me know if you are still having issues.