Opened 6 years ago

Closed 6 years ago

#9503 closed New Feature (fixed)

forms plugin adds context menu listener for unsupported input types

Reported by: mark Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0.1
Component: General Version: 4.0
Keywords: HTML5 Cc:

Description

I've been building a plugin to add file input fields into the content. Once a file input field is added, the context menu gives a "Text field properties" option. Within the Forms plugin, it appears there is a default case for the context menu listener that adds this menu option. This should be replaced with "case 'text':case 'password'" to prevent intercepting subsequent listeners for more specific field types. It could be extended to include the HTML5 types "numeric", "email" etc as well.

Attachments (1)

9503.patch (1.2 KB) - added by Alfonso Martínez de Lizarrondo 6 years ago.
Proposed patch

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by Jakub Ś

Keywords: HTML5 added
Status: newconfirmed
Type: BugNew Feature
Version: 3.6.54.0 (GitHub - master)
  1. This is not a bug but a feature request
  2. These new field types you talk about are HTML5 - http://www.w3schools.com/tags/att_input_type.asp
  3. CKEditor 3.x is XHTML 1.1 ("extended" HTML 4) compilant while CKEditor 4.x is intended to be fully HTML5 compilant thus this will probably be implemented in CKE 4.x

comment:2 Changed 6 years ago by mark

  1. I consider this a bug and not a feature request as I had to modify the ckeditor source to implement a file input field plugin with a functioning context menu. The change was however trivial - replace "default :" on line 169 of _source/plugins/forms/plugin.js with the supported types - "case 'text' : case 'password':".
  1. The <input type="file"> is not a new HTML5 feature. (However The same issue that is causing problems with the file input type could cause problems with these HTML5 types as well, so fixing this the right way would also allow support for HTML5 elements in plugins).
  1. This doesn't really help in supporting file inputs via a CKeditor plugin, but it is good to see that HTML5 support is on the roadmap! I might give CKeditor 4 beta a try.

comment:3 Changed 6 years ago by Jakub Ś

Note: besides input type="file" all field types available in HTML4 are handled by form plugin in v3 and v4.

There were new field types introduced in HTML5. Since CKEditor 4.x is meant to be HTML5 compliant this feature should have version 4 set (I doubt it will be implemented in v3).

This is not a regression but something that will probably be implemented only in v4.

comment:4 Changed 6 years ago by mark

Note: input type='file' is incorrectly handled by the form plugin.

Ignoring any HTML5 input fields, the form plugin prevents writing your own plugin to handle the HTML4 file input. The fix for file inputs is trivial (and might also allow some HTML5 input support to be added on using plugins as an added bonus).

Unless maybe I'm missing something and it is possible for a plugin to override a core plugins context menu listeners I can't see why this change would not be for the better.

Changed 6 years ago by Alfonso Martínez de Lizarrondo

Attachment: 9503.patch added

Proposed patch

comment:5 Changed 6 years ago by Alfonso Martínez de Lizarrondo

I've used this ticket as a little test to check if I've understood the way to do pull requests and branches in v4.

The patch attached here fixes the problem for v3 and it should be safe to add it, and https://github.com/ckeditor/ckeditor-dev/pull/11 tries to do the same thing for v4 (although I understand that if the bug is fixed in v3 then the patch will be merged along with all the other v3 fixes, so it will be another step with git: learn how to discard a rejected pull request)

comment:6 Changed 6 years ago by Alfonso Martínez de Lizarrondo

Owner: set to Alfonso Martínez de Lizarrondo
Status: confirmedreview

comment:7 Changed 6 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.0.1

comment:8 Changed 6 years ago by Piotrek Koszuliński

Owner: changed from Alfonso Martínez de Lizarrondo to Piotrek Koszuliński

Pushed git:cf006be (t/9503@cksource).

I added support for tel, email, search and url types to Alfonso's patch. These inputs are basically textfields, so they can be inserted with the current textfield dialog.

Other input's types have to handled by new dialogs in forms plugin or completely new plugins.

comment:9 Changed 6 years ago by Olek Nowodziński

Force pushed rebased branch.

comment:10 Changed 6 years ago by Olek Nowodziński

Status: reviewreview_failed

t/9503 is alright, but it'd be great to have tests for it. At the moment, tests basically check whether inputs are being correctly created. Testing this feature in the opposite direction would also be beneficial: select text field, open dialog and validate if attributes have been correctly imported into dialog logic.

comment:11 Changed 6 years ago by Piotrek Koszuliński

Status: review_failedreview

Pushed t/9503@tests.

comment:12 Changed 6 years ago by Olek Nowodziński

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Masterised git:45dd07d.

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