Ticket #4056 (closed Bug: fixed)

Opened 5 years ago

Last modified 4 years ago

Hidden field problem

Reported by: garry.yao Owned by: m.nguyen
Priority: Normal Milestone: CKEditor 3.3
Component: General Version:
Keywords: Confirmed Review+ Cc: matti.jarvinen@…, matthewlefflercomputer@…

Description (last modified by garry.yao) (diff)

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

forms_changes.zip (8.4 KB) - added by matti 4 years ago.
Form plugin changed files for comment
4056.patch (5.1 KB) - added by m.nguyen 4 years ago.
4056_2.patch (5.4 KB) - added by m.nguyen 4 years ago.
4056_3.patch (4.5 KB) - added by m.nguyen 4 years ago.
For hiddenfield

Change History

comment:1 Changed 5 years ago by fredck

  • Milestone changed from CKEditor 3.0 to CKEditor 3.1

comment:2 Changed 5 years ago by garry.yao

  • Keywords IE removed
  • Description modified (diff)

Accumulate Firefox problem.

comment:3 Changed 5 years ago by fredck

We should have something similar to the V2 solution here.

comment:4 Changed 4 years ago by garry.yao

  • Keywords Confirmed added
  • Summary changed from [IE6] Hidden field problem to Hidden field problem
  • Milestone changed from CKEditor 3.1 to CKEditor 3.x

comment:5 follow-up: ↓ 6 Changed 4 years ago by matti

  • 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 4 years ago by fredck

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 4 years ago by matti

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 4 years ago by matti

Form plugin changed files for comment

comment:8 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.x to CKEditor 3.3

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

comment:9 Changed 4 years ago by matti

@fredck I used fake element for hidden field.

comment:10 Changed 4 years ago by alfonsoml

  • Keywords HasPatch added

#4965 has been marked as dup

comment:11 Changed 4 years ago by mattleff

  • Cc matthewlefflercomputer@… added

Changed 4 years ago by m.nguyen

comment:12 Changed 4 years ago by m.nguyen

Thank matti.

comment:13 follow-up: ↓ 16 Changed 4 years ago by m.nguyen

  • Keywords Review? added
  • Owner set to m.nguyen
  • Status changed from new to assigned

comment:14 follow-up: ↓ 15 Changed 4 years ago by 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).

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 4 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 4 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 follow-up: ↓ 19 Changed 4 years ago by 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.

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 4 years ago by m.nguyen

comment:18 Changed 4 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:19 in reply to: ↑ 17 Changed 4 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 4 years ago by m.nguyen

For hiddenfield

comment:20 Changed 4 years ago by m.nguyen

  • Keywords Review? added; Review- removed

comment:21 follow-up: ↓ 22 Changed 4 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 4 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 4 years ago by m.nguyen

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

Fixed with [5295].

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