Opened 8 years ago

Closed 8 years ago

#4056 closed Bug (fixed)

Hidden field problem

Reported by: Garry Yao Owned by: Minh Nguyen
Priority: Normal Milestone: CKEditor 3.3
Component: General Version:
Keywords: Confirmed Review+ Cc: matti.jarvinen@…, matthewlefflercomputer@…

Description (last modified by Garry Yao)

There were many problems with hidden fields.

Reproducing Procedures

  1. Open the "replace by class" page, insert a form;
  2. Insert a hidden field with all attributes specified
    • Actual Result in Firefox: The inserted is not visible.
  3. Right click the hidden field to open context menu
    • Actual Result: The hidden field property entry is missing
  4. Click to open the hidden field dialog again;
    • Actual Result: All the attributes of it are lost.

Attachments (4)

forms_changes.zip (8.4 KB) - added by Matti Järvinen 8 years ago.
Form plugin changed files for comment
4056.patch (5.1 KB) - added by Minh Nguyen 8 years ago.
4056_2.patch (5.4 KB) - added by Minh Nguyen 8 years ago.
4056_3.patch (4.5 KB) - added by Minh Nguyen 8 years ago.
For hiddenfield

Download all attachments as: .zip

Change History (27)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.1

comment:2 Changed 8 years ago by Garry Yao

Description: modified (diff)
Keywords: IE removed

Accumulate Firefox problem.

comment:3 Changed 8 years ago by Frederico Caldeira Knabben

We should have something similar to the V2 solution here.

comment:4 Changed 8 years ago by Garry Yao

Keywords: Confirmed added
Milestone: CKEditor 3.1CKEditor 3.x
Summary: [IE6] Hidden field problemHidden field problem

comment:5 Changed 8 years ago by Matti Järvinen

Cc: matti.jarvinen@… added

FF doesn't show context menu for select, radio and checkbox and those cannot be selected by clicking either.

Fix in V2 replaces the checkbox, radio button and select box with clickable placeholders.

Could you give me a short description on how to make fake elements with CKEditor and how to use them? Pseudo code example will do (just to get me on the right track) so I can asses how much work would go into fixing this.

comment:6 in reply to:  5 Changed 8 years ago by Frederico Caldeira Knabben

Replying to matti:

Could you give me a short description on how to make fake elements with CKEditor and how to use them? Pseudo code example will do (just to get me on the right track) so I can asses how much work would go into fixing this.

Take a look at the "link" plugin, which creates fake elements for anchors. It should give you some hints on this.

comment:7 Changed 8 years ago by Matti Järvinen

I don't have svn version at hand so no patches here only changed files. Fixes radio, checkbox and hiddenfield with fake elements. Images are from V2 fix.

Few questions:

  • Is creation of new html element required every time ( Always create a new anchor, because of IE BUG.)?
  • How could this be redisigned to use original field commit methods?
  • Any thoughts on select element, how that should be fixed for FF?

Comments please. We really would appreciate this to be fixed in next release.

Changed 8 years ago by Matti Järvinen

Attachment: forms_changes.zip added

Form plugin changed files for comment

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.xCKEditor 3.3

We should have a fake element for hidden fields (V2) (@matti, I didn't look at your changes).

comment:9 Changed 8 years ago by Matti Järvinen

@fredck I used fake element for hidden field.

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

Keywords: HasPatch added

#4965 has been marked as dup

comment:11 Changed 8 years ago by Matthew Leffler

Cc: matthewlefflercomputer@… added

Changed 8 years ago by Minh Nguyen

Attachment: 4056.patch added

comment:12 Changed 8 years ago by Minh Nguyen

Thank matti.

comment:13 Changed 8 years ago by Minh Nguyen

Keywords: Review? added
Owner: set to Minh Nguyen
Status: newassigned

comment:14 Changed 8 years ago by Matti Järvinen

I'd would prefer to see someone fix this so that it uses setup and commit (easier to modify if needed for some reason).

Notice that my original zip has fixes for radio and checkbox too (which cannot be modified once created atleast on FF3.6).

comment:15 in reply to:  14 Changed 8 years ago by Garry Yao

Replying to matti:

I'd would prefer to see someone fix this so that it uses setup and commit (easier to modify if needed for some reason).

I can't see how this could improve maintainability.

Notice that my original zip has fixes for radio and checkbox too (which cannot be modified once created atleast on FF3.6).

We'll have a new ticket to discuss the trade-off to apply this also to checkbox etc.

comment:16 in reply to:  13 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Replying to m.nguyen: More efforts is needed to make it work with editor during I/O.

comment:17 Changed 8 years ago by Matti Järvinen

Replying to garry.yao:

I can't see how this could improve maintainability.

Sorry I should have been more precise what I ment was that patch doesn't use commit and setup functions of dialog definition and it makes it hard to add more dialog properties for the input by extending if needed.

For example we use additional required attribute for checkbox and some other inputs ( that is parsed within our cms when displaying, posting and receiving the form ). Without dialogdefinition.contents[].commit and setup functions we have to overwrite all of the onOK function to achieve this.

Replying to garry.yao: zip has this on plugin.js line 225 -> 274 though needs some work if radio and checkbox is not handled at the same time.

Changed 8 years ago by Minh Nguyen

Attachment: 4056_2.patch added

comment:18 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:19 in reply to:  17 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Replying to matti:

Replying to garry.yao:

I can't see how this could improve maintainability.

Sorry I should have been more precise what I ment was that patch doesn't use commit and setup functions of dialog definition and it makes it hard to add more dialog properties for the input by extending if needed.

I got your points, it would be better if fields commit directly to the real element while the fake elements are created from it at end, transparently from the user.

Changed 8 years ago by Minh Nguyen

Attachment: 4056_3.patch added

For hiddenfield

comment:20 Changed 8 years ago by Minh Nguyen

Keywords: Review? added; Review- removed

comment:21 Changed 8 years ago by Garry Yao

Keywords: Review+ added; HasPatch Review? removed

Please cleaning up coding styles and make sure the fake element image file is committed, also note that the commit should fall into to 3.2.x branch instead of trunk.

comment:22 in reply to:  21 Changed 8 years ago by Garry Yao

Replying to garry.yao:

Please cleaning up coding styles and make sure the fake element image file is committed, also note that the commit should fall into to 3.2.x branch instead of trunk.

Typo: 3.3.x branch.

comment:23 Changed 8 years ago by Minh Nguyen

Resolution: fixed
Status: assignedclosed

Fixed with [5295].

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