Opened 6 years ago

Closed 4 years ago

#8031 closed Bug (fixed)

handle textarea[required] attributes

Reported by: groovecoder Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.2
Component: General Version:
Keywords: HTML5 Drupal Cc:

Description

Calling $("#textarea").ckeditor(...) needs to handle textarea elements with HTML5 "required" attributes. It should remove the required attribute and optionally move it to the corresponding ckeditor element.

Attachments (2)

html5.html (703 bytes) - added by Jakub Ś 6 years ago.
html5_2.html (1.1 KB) - added by Piotrek Koszuliński 5 years ago.

Download all attachments as: .zip

Change History (24)

Changed 6 years ago by Jakub Ś

Attachment: html5.html added

comment:1 Changed 6 years ago by Jakub Ś

Status: newconfirmed

When textarea has attribute required and it's initial value is empty, like in the attached TC, then submitting the form is impossible.

Removing required attribute from textarea will probably solve the problem.

About putting required attribute to corresponding CKEditor element. I think that better option is just by checking with JS whether editor is empty or not.

comment:2 Changed 6 years ago by Jakub Ś

Keywords: HTML5 added

comment:3 Changed 5 years ago by don_busi

subscribing to this, as this bug bothers me too. :-(

comment:4 Changed 5 years ago by kapooostin

Still no news on this?

comment:5 Changed 5 years ago by Alex Verbruggen

First of all I want to congratulate CKEditor with making it in Drupal core :). Now I'm also interested in getting this HTML5 related bug finally fixed.

This issue is more than 1,5 years old. On Drupal.org we have a similar issue going that's forgotten: http://drupal.org/node/1338956

In my eyes this is a very critical bug since it's impossible for modern browser to submit a required textarea that's using CKEditor. I hope you can make the priority of this issue a little more important...

Last edited 5 years ago by Alex Verbruggen (previous) (diff)

comment:6 Changed 5 years ago by Théodore Biadala

It's actually a critical Drupal core bug now: see http://drupal.org/node/1954968

Last edited 5 years ago by Théodore Biadala (previous) (diff)

comment:7 Changed 5 years ago by Théodore Biadala

Keywords: Drupal added

comment:8 Changed 5 years ago by Wim Leers

I did some research in this area to solve this — because there are some arguments to be made that this should be solved in Drupal, not in CKEditor. I came up with essentially two options: http://drupal.org/node/1954968#comment-7253716

  1. it's possible to manually check the validity just before the submit event itself occurs and if that triggers the (new) invalid event, we can still do the serialization.
  1. But that doesn't solve the entire problem: what if CKEditor was in fact really left empty by the end-user? Then we're back to square one, where the browser either displays the HTML5 validation error message in the wrong location (e.g. in Firefox) or not at all (e.g. in Chrome). AFAICT this is an unsolvable problem, because HTML5 doesn't allow us to actually fix this. Solution: strip the "required" attribute if it exists and when CKEditor is applied to a <textarea>.

See the Drupal issue for (Drupal) patches/code.

comment:9 Changed 5 years ago by Théodore Biadala

I seem to get something working, taking the use-case from the attached file:

var orgTextarea = document.querySelector('#editor1');
orgTextarea.addEventListener('invalid', function (e) {
  e.preventDefault(); 
  
  // Fill in the original textarea
  
  // Check whatever else is constraints are on the field.
  if (e.target.checkValidity()) {
    e.target.form.submit(); 
  }
}, true);

Pretty crude code it's whatever worked first haven't dug too much yet. Seems to be making Firefox crash, that was pretty funny :)

I'd say there is a bit more hope than #8 would lead to believe.

comment:10 Changed 5 years ago by Wim Leers

I don't see how that solves the problem I pointed out in #8.2.

comment:11 Changed 5 years ago by Théodore Biadala

Doesn't solve point 2 but it does give a chance to solve point one.

About point 2 I tested on IE10. It fires the 'invalid' event but doesn't show a browser error nor console error.

comment:12 Changed 5 years ago by Wim Leers

Yes, point 1 is definitely solvable, more details, links and sample code at http://drupal.org/node/1954968#comment-7253716 :)

comment:13 Changed 5 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

Thanks guys for doing this research :) That saved me a lot of time.

I propose to fix this issue in CKE this way.

  • Remove 'required' attribute from textarea (it should be correctly restored when destroying editor) to prevent those issues with native tooltips.
  • Listen on submit and if data is required and editor is empty, then we should fire editor#required or editor#invalid event (although it'd be cool if someone found more suitable event name :D).
  • By default I'm not planning to do anything more (open alert or block form submitting), because we could break backward compatibility. However we can e.g. allow to block form submitting by setting returning false - IMO it will be useful option:
editor.on( 'required', function( evt ) {
    app.showError( 'Article content is required' );
    return false;
} );

In the future we can add more options, but for now this should be sufficient.

Does this sound good?

Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:14 Changed 5 years ago by Théodore Biadala

That sounds like a good compromise to me.

comment:15 Changed 5 years ago by Piotrek Koszuliński

Status: assignedreview

Pushed t/8031 on dev and tests and uploaded html5_2.html with a feature showcase.

Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

Changed 5 years ago by Piotrek Koszuliński

Attachment: html5_2.html added

comment:16 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.2

comment:17 Changed 4 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

There is only a minor simplification to be done in the code.

"require" is a boolean attribute, so we don't need to check or save it's value. Calling hasAttribute is enough.

That's totally logic, ofc... being it a boolean attribute means that a field with required="false" is... required! :| (WTFs echoing)

comment:18 Changed 4 years ago by Olek Nowodziński

Owner: changed from Piotrek Koszuliński to Olek Nowodziński
Status: review_failedassigned

comment:19 Changed 4 years ago by Piotrek Koszuliński

Owner: changed from Olek Nowodziński to Piotrek Koszuliński

comment:20 Changed 4 years ago by Piotrek Koszuliński

Status: assignedreview

Pushed both branches rebased and with required attribute simplification proposed by Fred. I chose to recover required="required" which is the cleanest option IMO.

comment:21 Changed 4 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

comment:22 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed with git:385a822 on dev and c2f8d2c on tests.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy