Make WordPress Core

Opened 6 years ago

Last modified 5 years ago

#44284 new defect (bug)

Remove e.preventDefault(); code line from widgets.js file

Reported by: alexvorn2's profile alexvorn2 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.6
Component: Widgets Keywords: has-screenshots has-patch needs-testing
Focuses: Cc:

Description

In wp-admin/js/widgets.js file we have some of this function that stops to display form validation errors on the page for widgets inputs...

We need to remove these function.

Attachments (2)

Untitled-2 copy.png (25.0 KB) - added by alexvorn2 6 years ago.
see example
widgets.js.patch (796 bytes) - added by alexvorn2 6 years ago.
Please check this patch, thanks!

Download all attachments as: .zip

Change History (12)

@alexvorn2
6 years ago

see example

#1 @ianbelanger
6 years ago

  • Keywords has-screenshots reporter-feedback added

Hi @alexvorn2, thanks for submitting your ticket.

Could you please share more information as to why we need to remove e.preventDefault() from the wp-admin/js/widgets.js file? Is there is specific reason that you feel it should be removed? Is it causing an error?

Thank you

#2 @alexvorn2
6 years ago

It does not cause an error but the problem is that I can not save the widget settings, if I have an input with min value = 1, and if I insert value 0..

see this example:
<input type="number" name="name_here" min="1">

if I insert into the input field "0", then the widget will not be saved and no notification will show because we have e.preventDefault(); function that blocks notifications...

The line to be removed in file wp-admin/js/widgets.js is 182.

<?php
} else if ( target.hasClass('widget-control-save') ) {
        wpWidgets.save( target.closest('div.widget'), 0, 1, 0 );
        e.preventDefault();

#3 @alexvorn2
6 years ago

e.preventDefault(); needs to be removed.

#4 follow-up: @woodent
6 years ago

@alexvorn2 The message you are seeing has nothing to do with the JavaScript. It is a browser based validation that is occurring because the input field has a min value of 1 applied in the HTML. If this is a custom widget you are creating, you should be able to change the min attribute in the HTML to 0 if you want to allow a 0 value.

#5 in reply to: ↑ 4 @alexvorn2
6 years ago

Replying to woodent:

@alexvorn2 The message you are seeing has nothing to do with the JavaScript. It is a browser based validation that is occurring because the input field has a min value of 1 applied in the HTML. If this is a custom widget you are creating, you should be able to change the min attribute in the HTML to 0 if you want to allow a 0 value.

You don't understand!
The screenshot is after I remove this function "e.preventDefault();" ... if it remains no message shows, so It creates confusion for users that click the save button and nothing happens because the validation fails... and the widget is not saved... I suggest to remove this function so the notification will show (as in the screenshot) ...

#6 @alexvorn2
6 years ago

I don't want to change min value to 0, I want the experience to be better for all users that use custom widgets and their validation fails!

#7 follow-up: @woodent
6 years ago

@alexvorn2 Ah, but the e.preventDefault() is there to prevent the form from actually being submitted. The JavaScript handles updating the widget and allowing the form to submit would cause the page to reload and the user's state on the widget page would be lost.

#8 in reply to: ↑ 7 @alexvorn2
6 years ago

Replying to woodent:

@alexvorn2 Ah, but the e.preventDefault() is there to prevent the form from actually being submitted. The JavaScript handles updating the widget and allowing the form to submit would cause the page to reload and the user's state on the widget page would be lost.

I tested and everything is fine for me, no page reload, if you think this function should stay - maybe move to other place (ex: after validation check)...

@alexvorn2
6 years ago

Please check this patch, thanks!

#9 @alexvorn2
6 years ago

  • Keywords has-patch needs-testing added

#10 @ianbelanger
5 years ago

  • Keywords reporter-feedback removed
Note: See TracTickets for help on using tickets.