Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7915]Fix HTML5 validation fails on resource uploads #7925

Merged
merged 1 commit into from Nov 24, 2023

Conversation

TomeCirun
Copy link
Contributor

Fixes #7915

Proposed fixes:

After debugging for a while, I found two potential solutions for the issue:

1). Change Input Type (Implemented in my PR): This approach involves altering the type of the hidden input to text, effectively bypassing HTML5 validation. The type reverts to 'url' when the upload is cleared, and the 'LINK' option is chosen.

2). Removing this check: An alternative option is to eliminate the "for_edit" check, allowing the complete qualified URL to be returned.

Please let me know your preference for the approach to proceed with.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

@ThrawnCA
Copy link
Contributor

ThrawnCA commented Nov 22, 2023

I would lean toward making the least change, so apply url validation only if we actually expect a URL, the way you have implemented here.

This way, the field contents are the same as they were before, we just partially roll back the extra validation that 2.10 added.

@ThrawnCA
Copy link
Contributor

You need a changelog entry to make towncrier happy. Do you want to just cherry-pick qld-gov-au@097e77d ?

@amercader amercader self-assigned this Nov 23, 2023
@amercader amercader merged commit 16727f7 into ckan:master Nov 24, 2023
7 of 8 checks passed
@amercader
Copy link
Member

Thanks @ThrawnCA and @TomeCirun, I added the changelog entry

@Chealer
Copy link

Chealer commented Feb 29, 2024

Thank you very much @TomeCirun and @amercader. This was backported to 2.10 via 2d60c7f and fixed in CKAN 2.10.3.

@Chealer
Copy link

Chealer commented Mar 1, 2024

I would argue the proper fix would be neither, but rather to disable the URL field. Performance-wise, this is best, as a field which is not being modified does not require transmission. In fact, the best would be to not even include that field in the form, as it is superfluous.

@Chealer
Copy link

Chealer commented Mar 5, 2024

I tried to cleanup the current workaround with the following:

// Workaround #7915 (HTML5 validation silently fails on some resource uploads)
$( "#resource-edit" ).on( "submit", function( event ) {
    if ($('#resource-url-upload').prop('checked')) {
      // Make the irrelevant URL field type "text" rathen than "url", so the form can be submitted even if the field's value is not a valid URL
      jQuery('#field-resource-url').attr('type', 'text');
    }
});

This unfortunately fails (does nothing) because the submit event is fired after built-in form validation.
A clean workaround would require an event fired before form submission, which doesn't exist.

The following could work, but there may be browsers where click doesn't always fire before submit:

// Workaround #7915 (HTML5 validation silently fails on some resource uploads)
$( "#resource-edit" ).on( "click", function( event ) {
  // If the resource is not a link, make the irrelevant URL field's type "text" rathen than "url", so that the form can be submitted even if the field's value is not a valid URL.
  jQuery('#field-resource-url').attr('type', $('#resource-url-upload').prop('checked') ? 'text' : 'url');
});

If the current approach is kept, it would at least clarify to comment the added workaround with something like:

// Begin workaround #7915 (HTML5 validation silently fails on some resource uploads)
      // Change input type to "text" if Upload is selected, to disable URL validation
      if ($('#resource-url-upload').prop('checked')) {
        urlField.attr('type', 'text');
      }

      // revert to "url" when the user selects the Link option
      $('#resource-link-button').on('click', function() {
        urlField.attr('type', 'url');
      }) 
// End workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTML5 validation silently fails on some resource uploads
4 participants