Ticket #8031 (closed Bug: fixed)

Opened 3 years ago

Last modified 15 months ago

handle textarea[required] attributes

Reported by: groovecoder Owned by: Reinmar
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

html5.html (703 bytes) - added by j.swiderski 3 years ago.
html5_2.html (1.1 KB) - added by Reinmar 18 months ago.

Change History

Changed 3 years ago by j.swiderski

comment:1 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed

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 3 years ago by j.swiderski

  • Keywords HTML5 added

comment:3 Changed 2 years ago by don_busi

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

comment:4 Changed 22 months ago by kapooostin

Still no news on this?

comment:5 Changed 21 months ago by alexverb

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 21 months ago by alexverb (previous) (diff)

comment:6 Changed 18 months ago by nod_

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

Last edited 18 months ago by nod_ (previous) (diff)

comment:7 Changed 18 months ago by nod_

  • Keywords Drupal added

comment:8 Changed 18 months 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 18 months ago by nod_

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 18 months ago by Wim Leers

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

comment:11 Changed 18 months ago by nod_

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 18 months 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 18 months ago by Reinmar

  • Status changed from confirmed to assigned
  • Owner set to Reinmar

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 18 months ago by Reinmar (previous) (diff)

comment:14 Changed 18 months ago by nod_

That sounds like a good compromise to me.

comment:15 Changed 18 months ago by Reinmar

  • Status changed from assigned to review

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

Last edited 18 months ago by Reinmar (previous) (diff)

Changed 18 months ago by Reinmar

comment:16 Changed 16 months ago by Reinmar

  • Milestone set to CKEditor 4.2

comment:17 Changed 16 months ago by fredck

  • Status changed from review to review_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 15 months ago by a.nowodzinski

  • Status changed from review_failed to assigned
  • Owner changed from Reinmar to a.nowodzinski

comment:19 Changed 15 months ago by Reinmar

  • Owner changed from a.nowodzinski to Reinmar

comment:20 Changed 15 months ago by Reinmar

  • Status changed from assigned to review

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 15 months ago by fredck

  • Status changed from review to review_passed

comment:22 Changed 15 months ago by Reinmar

  • Status changed from review_passed to closed
  • Resolution set to fixed

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

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