After upgrading to the newly released 3.4 (from 3.3) the bulk operation buttons no longer work when clicked..

Reverting back to 3.3 all works fine again..

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wipeout_dude created an issue. See original summary.

joelpittet’s picture

Try selecting items to do the action. They are disabled until you can do something so people don't do an action on nothing.

I'm assuming that is what you are seeing, if not please explain and tell me about your environment.

joelpittet’s picture

Status: Active » Postponed (maintainer needs more info)
alh’s picture

I may have the same issue. I think that in my case, it is because there are two VBO forms on the same page. In Ubercart, we have a date range and an operation (print/delete). The error message is shown after selecting two orders for print. Perhaps VBO is confused as the first form isn't completed as we didn't select a date range. Just guessing however. Also got an error message on install which I will post separately. Will pass along any further clues as I look at it more closely. Great module though - thank you.

I know this isn't too helpful but I can send you the image if you like with the forms and error message.

Orders
Status message Operating in maintenance mode. Go online.
Error message Please select at least one item.
View order number

View by status

Date (uc_orders)

Month

Day

Year

Apply
Operations
Execute
Actions Order IDsort ascending Customer Total Purchase date Status

View order 5524.Print order 5524.Edit order 5524.Delete order 5524.Package order 5524 products.Ship order 5524 packages. 07/08/2015 Completed

View order 5523.Print order 5523.Edit order 5523.Delete order 5523.Package order 5523 products.Ship order 5523 packages. 07/08/2015 Completed

joelpittet’s picture

@alh both of those sound like separate issues but there could be a bug with two vbo views on a page that has to do with scoping the selectors. Not sure if that is the same as the the original post though.

alh’s picture

Hi Joel - yes, I accept that as a possiblity. Do you want me to generate a separate issue?
Al

alh’s picture

Sorry - I should add that if we do select a date range the bulk operation works perfectly. So it is likely as you suggested - the module may only be looking at the first form.
Al

wipeout_dude’s picture

Title: Bulk operation buttons don't work after upgrade to 3.4. » Bulk operation buttons don't work after upgrade to 3.4 when Force Single selected.

Ok, I have done some testing and it seems the issue is related to views where "Force single" is selected.. As soon as I deselected that the buttons worked.. As it happens a number of our views use "Force Single" which is why they all stopped working after the upgrade..

I have updated the issue title to be more relevant to the bug.

wipeout_dude’s picture

Status: Postponed (maintainer needs more info) » Active
joelpittet’s picture

@alh please file a separate issue if it's two VBO forms on a page causing issues as that could be fairly straight forward to fix.

@wipeout_dude thank's for clarifying the problem and narrowing it down.

joelpittet’s picture

scottsawyer’s picture

I experienced the same issue, one vbo field with multiple operations. Could not click a button even after selecting a row. Maybe related, I have a filter that uses ajax, only tested after filtering my list of 300+ records.

Rolled back to 3.3 and seems to work.

hgoto’s picture

As far as I understood, there may be several possibilities of patterns where this problem happens.

- A. Multiple VBO forms in a page.
- B. "Force single" option is used.
- C. ajax is used.

As for A, the contexts are already separated for each form. But I'm not sure and I'll try to reproduce it. As for B, I confirmed that B makes the problem and found the code to correct.

As for C, @scottsawyer, could you please tell me a little more about what you think is the cause and condition?

hgoto’s picture

As for B, this patch should work. (I don't think this patch is enough for all the cases and this is just a first trial :) )

joelpittet’s picture

Status: Active » Needs review

I think C may be a red herring, but we can wait for @scottsawyer to provide steps to reproduce.

Thanks for the quick patch to B.

@alh may be doing A in a new issue if there are steps to reproduce too.

joelpittet’s picture

@wipeout_dude could you test the patch out in #14 and let us know if it resolves the issue.

stefanos.petrakis@gmail.com’s picture

Good evening,

Sorry for the interference, but, I think that #14 is a bit frail.
Like in #2855782: Disable submit button works only after a second selection is done: Recent jQuery+WholeRowClickable, there are other configurable behaviours that the VBO javascript has to take into account,
like clicking on a row. And that, as correctly noted in #14 would mean more cases to cover.

A simpler way to make this work would be to add the class 'vbo-select' to the radios,
which semantically makes sense AFAIAC, since the intended action is that of 'selecting' a row.

Here is a minimal-change patch that should solve this issue.

hgoto’s picture

@stefanos.petrakis thanks for the review! As you tell, we need to recognise and cover possible patterns.

Adding the class 'vbo-select' to the radios is simpler but may not be good since there's a logic only for checkboxes (not for radios) like the following:

    $('.vbo-select', form).click(function() {
      // If a checkbox was deselected, uncheck any "select all" checkboxes.
      if (!this.checked) {
        $('.vbo-select-this-page', form).prop('checked', false);
        $('.vbo-select-all-pages', form).prop('checked', false);
        // Modify the value of the hidden form field.
        $('.select-all-rows', form).val('0')
stefanos.petrakis@gmail.com’s picture

Hey @hgoto,

That's a good point, maybe the code needs to be more verbose to be more understandable/maintainable.
Still, in this case, the condition if(!this.checked) and following code-block will only work for checkboxes and will never be used for radios, since you cannot un-check a radio by clicking on it.

hgoto’s picture

@stefanos.petrakis, I see. Surely in this case, the if condition always becomes false for radios!

I almost thought the patch is OK but worry a little about another side effect of the class change. If a developer uses 'vbo-select' as a CSS selector so that only selectboxes should be decorated, adding 'vbo-select' to radio buttons causes a design change, doesn't it? Adding an already used class to another elements is like an API change for front end, I feel... Is my understanding correct?

scottsawyer’s picture

You may be right that it's a red herring. I was reading a different queue and saw someone mention ajax. Sorry, looking at too many issues at once.

I've rolled back to 3.3, things seem to be working. It's going to be a few days before I can test this patch.

Our case is always force single, so that may be the culprit.

stefanos.petrakis@gmail.com’s picture

@hgoto: Sound argumentation, thanks for the feedback.

I would still insist a tiny bit more and say that .vbo-select as a front-end API reserved word does not uniquely identify a checkbox, or a radio for that matter. I think it's more generic and could cover both.

Description-wise, .vbo-checkbox would be the better term to uniquely identify a checkbox element (following the FAPI terminology).

We just entered the words domain :-)

hgoto’s picture

@stefanos.petrakis: thank you for your response and detailed explanation. I see!

I worried about the class change just because I myself used the CSS selector .vbo-select to adjust the design of checkboxes in a VBO table in my previous project. At the moment, I didn't have an idea that the selector will be applied also to VBO radio buttons in the future.

So I thought that there's a chance that existing sites' design of VBO radio buttons would be changed with the approach of the patch #17 (if the site's theme uses the class .vbo-select as a CSS selector).

If I understand it correctly, the both approaches make sense and work well in respect of the problem B in #13. The difference is that the patch #17 prioritizes the code simplicity by unifying the logic of checkboxes and radio buttons. On the other hand, the patch #14 prioritizes backward compatibility for style rather than the code simplicity.

I also have a motivation to make the code simpler, but I'm not sure which value we should prioritize. I'd like other opinions and reviews :)

---

@scottsawyer: thank you for your comment and explanation. If possible, please review the patches in your project.

stefanos.petrakis@gmail.com’s picture

Good summary @hgoto. I would also like to hear other people's opinions.

joelpittet’s picture

I'm for the class addition but can I suggest using a class that doesn't exist yet to avoid front-end issues. .vbo-select--radio for example since it's a variant on a the typical checkbox select?

acrosman’s picture

I'm having similar symptoms (actually better described in #2856944: 'Please select at least one item' - after upgrading to 7.x-3.4 getting the "Please select at least one item" notice). The patch in 17 had no effect. There is only one VBO on the page, no AJAX, nor is Force Single enabled.

Rolled back to 7.x-3.3 and the problem resolved.

hgoto’s picture

@acrosman, thank you for your report. Could you share the concrete steps to reproduce the problem?

bruceshort’s picture

@acrosman Yes I agree, this thread relates to force single - my view doesn't have force single enabled...

I've re-opened my issue at #2856944 'Please select at least one item' - after upgrading to 7.x-3.4

stefanos.petrakis@gmail.com’s picture

Hey @hgoto and other threaders, here are some more thoughts on this:

a) Introducing a new CSS class would cause a larger rewrite of the JS code than I would personally prefer. I think that the benefit of keeping the code base simple and unifying the terminology and code for both checkboxes and radios offers more advantages.
b) Design-BC breakage: I think that switching from a .vbo-select selector to .vbo-select[type=checkbox] or .vbo-select[type=radio] is a reasonable change and would make things more readable. Something that could be described shortly in the release notes as well.

A nice day to all!

joelpittet’s picture

@stefanos.petrakis attribute selectors sound reasonable to me considering it's not a select box anyways it's a "selection" of an option. Adding an extra class would effectively do a similar thing but I agree less JS changes. Feel free to post a patch on this.

stefanos.petrakis@gmail.com’s picture

@joelpittet: The patch in #17 still applies, I suggested using attribute selectors as a recommendation to CSS maintainers and authors. They can update their stylesheets using attribute selectors to continue targeting checkboxes only (and not radios). This is related to the situation @hgoto described in #23. I also suggested adding sth like this in the release notes, it if goes all the way to a stable, sth like:

To avoid CSS styles breaking (e.g. applying checkboxes specific styles to radios), one should update their existing styles to target .vbo-select elements using attribute selectors, for example:

Before:

.vbo-select { /* Styling checkboxes */ }

After:

.vbo-select[type=checkbox] { /* Styling checkboxes */ }

@hgoto: How do you see such a way to move forward with this?

hgoto’s picture

Status: Needs review » Reviewed & tested by the community

@stefanos.petrakis: I see. As with the possible style change, it would be enough to add some comments in the release notes.

I noticed that the following 2 selectors in the JS code will target completely same elements after applying the patch #17.

  • '.vbo-select'
  • 'input[id^="edit-views-bulk-operations"]'

So we can make the code cleaner by replacing all 'input[id^="edit-views-bulk-operations"]' with '.vbo-select'. However, I believe it's better to do that in a separated follow-up issue after fixing the problem.

I checked the patch #17 again with the latest HEAD and confirmed it works well. I'd like to mark this RTBC. Thank you, again :)

joelpittet’s picture

Version: 7.x-3.4 » 7.x-3.x-dev
Status: Reviewed & tested by the community » Fixed

@hgoto, yes please do create a follow-up so we can test that out.

Thanks @stefanos.petrakis for the release notes, I hope to remember to come back when creating a new release.

stefanos.petrakis@gmail.com’s picture

You are both welcome, will keep an eye on the release notes when they come out, luckily they are editable directly from d.o. after publication of a release (just checked).

hgoto’s picture

Thank you, @joelpittet, @stefanos.petrakis and all.

It's late but I filed a small follow-up issue according to the thought in #32. I'd like someone to review a patch there.

#2867407: 2 patterns of jQuery selector are used for the same elements in views_bulk_operations.js

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

webservant316’s picture

when will vbo 7.x-3.5 be released to include this patch?

joelpittet’s picture

Release plan is here: #2878878: Plan for VBO 7.x-3.5 release

I need help with reviews of the remaining tasks.