Problem/Motivation

Drupal 8 leverages the required attribute for html inputs. There is a known issue with the Chosen library where it breaks this (https://github.com/harvesthq/chosen/issues/2075), which hasn't been fixed, but includes workarounds.

To reproduce, add a required multi-select field to a content type. Fill out all other required fields on the node add form. The form will not submit, but no direction is given to the user.

In Chrome, the user gets no feedback, and this error appears in the console log:

An invalid form control with name='field_test[]' is not focusable.

In Firefox, the message appears at the top of the browser window, rather than by the element:

Proposed resolution

We should wrap some JS around required fields to handle this via one of the workarounds.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom created an issue. See original summary.

jhedstrom’s picture

Issue summary: View changes
FileSize
123.56 KB
jhedstrom’s picture

Status: Active » Postponed

If https://github.com/harvesthq/chosen/pull/2594 or something similar goes in, we won't have to address this here.

danquah’s picture

While we wait for a proper fix like https://github.com/harvesthq/chosen/pull/2594 I've attached a patch that rather crudely implements the fix @jhedstrom suggested in the github issue directly in the modules .js

danquah’s picture

The previous patch did not work in all cases, so here is a more simple patch that in case of a required multipe-selector removes the required attribute. This means you loose the client-side validation, but we will at least be able to submit the form.

This element will still be visually marked as required (red star), and will be highlited if it fails validation.

Again, this is a temporary workaround.

acbramley’s picture

Rerolled against latest dev.

gnuget’s picture

I just create a patch which use the workaround suggested on: https://github.com/harvesthq/chosen/issues/515#issuecomment-104602031

This make it work without need to remove validation.

mpp’s picture

#7 applies and rather 'brutally fixes' the issue :)

mpp’s picture

Note that the message appears a bit too low though. Edit: Probably because we have two consecutive elements that use the chosen widget.

gambry’s picture

#9 No, I have 2 chosen fields in a row as well. I moved one and message still appears lower then it should.

That's due the select multiple height (or width in some cases) being spanning over its chosen widget.
Attached patch set select height and width to 0, and widget looks now correct to me.

I'm also setting this to NR as it has been postponed for too long, and all this time there hasn't been any light at the end of bug's tunnel.
And I think we are going to wait still a good while before issue is fixed on the library itself.

Attached also a screenshot proving the html5 validator works, without the need to skip it.

gnuget’s picture

Hi Gambry.

Thanks for your review, can you please provide an interdiff?

Thanks!

gambry’s picture

FileSize
633 bytes

Yep, sorry. Got distracted and forgot.

Attached interdiff 7-10.

dpagini’s picture

Status: Needs review » Reviewed & tested by the community

We've been using this patch for 4 months now and it's fixed this issue for us, setting to RTBC status.

gnuget’s picture

hi @dpagini.

Which pacth did you use? it seems that the workaround on #7 has a few bugs when used on multiple elements, and seems that #10 fixed this problem (I haven't tested it myself)

Also, not sure if we can just commit a workaround as a fix.

Maybe we can do it if we create a follow-up remind us to undo this change when the library is fixed?

dpagini’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
35.71 KB

Was using #7 and updated to #10. I missed the few bugs with the "multiple elements" (didn't quite understand the comment completely). I have multiple chosen elements on the frontend, and the error message is showing up in an "acceptable" location for me (see screenshot). It's all the way to the left of the chosen select, but right in the middle. I have not testing on the backend. If I have a chance I'll enable it there and see how it works for me... I'll set back to Needs Review until I have the chance to do that or someone else reviews.

As for the committing a fix... I think it's silly to hold out for those fixes. Unless someone has the ear of the maintainer, there are so many PRs open for that library, it seems like it just doesn't get the active attention it needs. Just my $0.02.

Anas_maw’s picture

This workaround not working for me.

Anas_maw’s picture

After long search i found that it's an issue with Drupal core, https://www.drupal.org/project/drupal/issues/1797438
The last patch works fine for me after applying core patch

Anas_maw’s picture

Status: Needs review » Reviewed & tested by the community

This should be committed, waiting for chosen to fix this will not be the right choice.

DanieleN’s picture

+1 #18 (and Patch #10 with Core 8.5 works fine)

Problem with IE11

Steps for reproduce:

  1. Required Chosen Field is empty
  2. Click Save
  3. HTML5 Warning show up
  4. Select a value in the chosen field
  5. HTML5 Warning is current showing (Expected: Node can be saved. )

This is an important feature for using this module.

Update:
Works in IE11 with Core Patch against HTML Validation:
https://www.drupal.org/project/drupal/issues/1797438 (current #128)

nagy.balint’s picture

In #17 there is a core patch which seem to not be committed yet in 8.5
Is that not needed anymore?

sonfd’s picture

Patch #10 is resolving issues for me.

mpp’s picture

Anyone willing to commit #10?

nagy.balint’s picture

There is no response yet to #20.

If it needs a core patch to work then its a different story.

mpp’s picture

@nagy.balint #1797438 describes an issue in Drupal core with HTML5 validation. The current direction is to totally remove HTML5 validation, which eventually makes this issue irrelevant. But another JavaScript based approach that keeps HTML5 validation is also discussed.

As a maintainer you should decide if you want to support HTML5 validation or not. If you do, I'd merge this patch, if you don't I'd refer to the core issue and suggest people to disable HTML5 validation as a whole.

nagy.balint’s picture

Comment #17 says that the workaround did not work without the core patch.

Which is what i asked in #20.

mpp’s picture

Your answer to #20 is no, the core patch is not needed.

If one does apply the core patch, HTML5 validation is disabled. This would make this issue (to support HTML5) irrelevant.

  • nagy.balint committed 9ad5dac on 8.x-2.x authored by gambry
    Issue #2705891 by danquah, gambry, gnuget, acbramley, mpp, jhedstrom,...
nagy.balint’s picture

Status: Reviewed & tested by the community » Fixed

I guess we can add this temporarily.

Status: Fixed » Closed (fixed)

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