Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Follow up for #1938062-43: Convert the recent_comments block to a view
Problem/Motivation
Cannot submit a comment on a node.
Proposed resolution
TBD
Remaining tasks
- (optional) find the commit using git bisect that introduced this error.
- discuss a proposed resolution
- ?
Steps to reproduce
- add an article
- try either the save or preview button to add a comment.
User interface changes
No UI changes.
API changes
No api changes anticipated.
Original report by @jibran
In #1938062-43: Convert the recent_comments block to a view
While adding comment I am unable to press submit or preview button due JS error.
An invalid form control with name='comment_body[und][0][value]' is not focusable.
Dono about any active issue related this.
Comment | File | Size | Author |
---|---|---|---|
#44 | html5-ckeditor_1.jpg | 87.96 KB | Garra |
#44 | html5-ckeditor_2.jpg | 93.4 KB | Garra |
#34 | text_editor_html5_required-1954968-34.patch | 1.32 KB | Wim Leers |
#19 | option-a.patch | 2.06 KB | Wim Leers |
#19 | option-b.patch | 849 bytes | Wim Leers |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedComment #2
YesCT CreditAttribution: YesCT commentedAlso, when using devel to generate content with comments, I get:
Comment #3
jibranIt is a JS error and I think it has somthing to do with CKEditor so adding tags to get more eyes.
Comment #4
Wim LeersI'm pretty sure this (screenshot in #1) is not a JS error, but a HTML5 validation error.
Please try to reproduce it with JS disabled. If you still can reproduce it, then it's not a JS error :)
Comment #5
jibranIn firefox after disabling java script and after selecting "Restricted HTML" text format I am able to submit the form.
In chrome i am getting this error
and after disabling java script and after selecting "Restricted HTML" text format I am able to submit the form.
Comment #6
Wim LeersIndeed HTML 5 validation, as I suspected: http://stackoverflow.com/a/7264966.
This is probably not a JavaScript issue, and definitely not a CKEditor in core issue.
Comment #7
xjmComment #8
webchickThought I was going absolutely insane.
Comment #9
webchickBtw, Safari works. Chrome doesn't. Don't have any other browsers on this machine.
Comment #10
webchickHm. Well, something definitely seems CKEditor-related. If you switch to "Filtered HTML" with no CKEditor, the form submits fine. (thanks, andypost)
Comment #11
Damien Tournoud CreditAttribution: Damien Tournoud commentedBoth errors mean that the (hidden)
<textarea>
control is empty at the time of the submit. This probably means that the browser validation happened too early (before CKEditor could update the field), and it is a problem only in the case of the comment form, because it's the only required text area that we have.From #1, I conclude that this happens on both Firefox and Chrome (with slightly different symptoms), so retitling.
Comment #12
nod_tag
Comment #13
nod_upstream bug: http://dev.ckeditor.com/ticket/8031
Comment #14
alexverb CreditAttribution: alexverb commentedI also think this is a ckeditor issue. They just need to copy over the values to the actual textarea before any browservalidation can take place. I don't think it's Drupal's job to make all wysiwyg editors HTML5 compatible...
Duplicate issue on D7: http://drupal.org/node/1338956
Comment #15
Wim Leers#10, #11, #13 and #14 are all right. Apologies.
It's not a general JavaScript issue: it's CKEditor's responsibility (or rather: WYSIWYG editors in general, when used with forms that use HTML5 form validation). The WYSIWYG module issue that alexverb pointed to (#1338956: [upstream] HTML5 "required" attribute prevents forms from submitting) contains more info.
I wonder if it might be better to do what alexverb suggested in comment 7 of that issue: use the
novalidate
attribute whenever a Text Editor is attached to a required form item. (Should be possible to do that ineditor_pre_render_format()
.) Server-side validation would still kick in.I think that'd be worse, but I'm not entirely sure. Thoughts?
Comment #16
andypostwysiwyg should call detach() before html5 validation, so leaving JS tag
Is it possible to make custom code execution before internal validation?
For example, when I use "etherpad" as wysiwyg backend I have to make additional call to server to get actual content and sometimes I need to make some text processing before a content is sent to validator
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commentedI am not quite sure either that this is the responsibility of CKEditor itself. We do call updateElement() manually when serializing the form.
Comment #18
nod_Fact is that it's a bug for us and for them. Check the testcase in their bug tracker, it breaks simple (Drupal-less) HTML5 forms.
I marked this one as postponed since a side-effect of fixing CKEditor for all HTML5 forms would fix our bug too. I pinged the CKEditor guys and they told me that someone from their team will have a look at it. It's just a matter of who has time to dig in and fix it.
Comment #19
Wim Leers#16: that's a good point, but we're already doing that. In
editor.js
:The problem is that the browser's HTML 5 validation kicks in even before that. See http://stackoverflow.com/q/7587511.
Now, 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.See
option-a.patch
.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, see screenshot in #1) or not at all (e.g. in Chrome, see the OP). AFAICT this is an unsolvable problem, because HTML5 doesn't allow us to actually fix this.
See
option-b.patch
.Comment #20
Wim LeersComment #21
nod_I'd rather wait a bit to see what comes up from the ckeditor issue before we commit something to core.
Comment #22
nod_tag
Comment #23
alexverb CreditAttribution: alexverb commentedI think the "novalidate" attribute would be a valid option if we did not have to give in on conformity. Because doing this only for WYSIWYG fields would mean you can let the user go through 2 different validators (browser vs drupal).
Just think about getting pulled over by a shiny browserpopup that's telling you to fill in the title field. Submitting the form again, doing a page load and getting pulled over by Drupal's famous red warning you missed a WYSIWYG field. This would be just plain confusing...
If this issue doesn't get resolved by the editors (ckeditor, tinymce, ..). I say we should seriously think about disabling the HTML5 validation entirely. I know it would be pretty controversial to do this, but if you want an identical user experience for everyone it should at least be a configurable option (core, theme, contrib?). And in my opinion the WYSIWYG feature in core outweighs the benefits of browser validation.
If we do decide to fix this problem through Drupal or the editors we should also find a solution for the positioning problem of the browser validation popup. I'm shure there must be a CSS way to let the browser know where our textarea (wysiwyg) is at...
Comment #24
Wim LeersI forgot to mention:
novalidate
andnoformvalidate
(which is already present in Drupal 8 core whenever a submit button has the#limit_validation_errors
property, for e.g. "Previous" and "Add another item" buttons) apply to the entire form, not specific form items!So, this scenario is impossible by using those attributes:
That scenario is possible though by simply removing the
required
attribute from the hidden text area.And AFAICT there is zero hope of being able to do this:
(Which means HTML5 validation + WYSIWYG editors is an unsolvable problem until the spec makes it possible to be able to position HTML5 validation error messages.)
At the very least it's puzzling that they (the browser implementors and spec writers) did not take this into account explicitly.
Comment #25
webchickTagging HTML5 as well, in case those folks have any bright ideas.
Comment #26
aroq CreditAttribution: aroq commentedRelated issue: http://drupal.org/node/1961908
Comment #27
aroq CreditAttribution: aroq commentedAdded HTML 5 tag back.
Comment #28
aroq CreditAttribution: aroq commentedUpdated tag.
Comment #29
aroq CreditAttribution: aroq commentedupdating tags again.
Comment #30
YesCT CreditAttribution: YesCT commentedre #26: could #1961908: Comments can't be saved/submited be a duplicate of this?
Comment #31
nod_upstream moving: http://dev.ckeditor.com/ticket/8031#comment:13
Comment #32
Wim LeersUpstream moving based on our feedback/my patches in #19 :) Cross-project collaboration++
So… question then is: do we want to implement the same in editor.module to have it automatically apply to all WYSIWYG editors? Or do we leave the responsibility with the WYSIWYG editors, whom will have to solve this anyway? And if the latter, do we apply
option-b.patch
from #19 temporarily until we can pull in a version of CKEditor with the fix, to unbreak our comment form until then; or do we hotfix our CKEditor version?Comment #33
Damien Tournoud CreditAttribution: Damien Tournoud commentedOption (b) is fine. Client-side validation brings very little value to begin with in that case (why would you submit the comment form without a comment?), and the requirements are checked on the server-side anyway.
Comment #34
Wim LeersPrecisely :)
Rerolled
option-b.patch
to now also restore the "required" attribute when switching to a text format that doesn't use a Text Editor.Comment #35
jenlamptonPatch still applies cleanly, and I'm able to submit comments again. Thank you!
Comment #36
nod_+1 to RTBC
I don't think it should be the responsibility of drupal to take care of it but the specific editor implementation at least. Anyway let's see how it goes with other editors and the required attribute to see what we'll do with this bit of code.
Comment #37
alexpottCommitted 924be51 and pushed to 8.x. Thanks!
Comment #38
Wim Leers.
Comment #40
PanchoRetroactively tagging and moving this to editor.module because the workaround has been generically introduced there.
Which I actually believe should change after #2036253: Update CKEditor library to 4.2.
Comment #41
PanchoTags didn't stick.
Comment #42
PanchoAnd one more tag.
Comment #43
Garra CreditAttribution: Garra commentedHello and thanks for this help
but i have always the same problem
I have installed ckeditor module (last version 7.x-1.13) and ckeditor library 4.2 but the problem is still there
in Firefox, html5 required message is visible at the bottom of the window (even if i write something in my textarea field)
I don't understand your patch because i don't have "module/editors" but "module/ckeditor" or "module/wysiwyg" directories (before i tested with wysiwyg module)
so i don't know how this patch works
thanks a lot if you can help me?
Comment #44
Garra CreditAttribution: Garra commentedIn fact, what i found is :
1) The first time i do not write text i have the drupal core message (red message in the top of the page)
2) i write something and then i have the html5 message in the very bottom on the page (hidden buy the windows 7 bottom toolbar)
(i join images)
does anybody have the same problem?
could anybody can help me?
thanks a lot
Nora from Paris
Comment #45
Garra CreditAttribution: Garra commentedsorry
i wrote my comment on a 8.x-dev version post but i am in 7.23.
i am very sorry :-(
it was my firsts comments....
Comment #46
tim.plunkettThis was a core D8 bug that was fixed. If you're having this problem on Drupal 7, it is probably due to a contributed module.
Comment #47
YesCT CreditAttribution: YesCT commentedstandardizing tag. sorry for noise. (https://drupal.org/needs-tags)