Ticket #7893 (closed Bug: fixed)

Opened 6 years ago

Last modified 6 years ago

Enabling link by UID option in kupu breaks inline editing

Reported by: habbers Owned by: ree
Priority: major Milestone: 3.1.5
Component: KSS (Ajax) Version:
Keywords: kupu link by UIDs inline editing Cc: gotcha, mborche, tomster, maurits, grahamperrin, hiddenman, davby, russell84

Description

After upgrading to plone 3.0.6 I noticed that I got the following error when trying to save inline edits to the body text:

2008-02-21T17:43:09 ERROR Zope.SiteErrorLog  http://########:8081/wilderness/frontpage/saveField/saveField Traceback (innermost last):

Module ZPublisher.Publish, line 119, in publish
Module ZPublisher.mapply, line 88, in mapply
Module ZPublisher.Publish, line 42, in call_object
Module archetypes.kss.fields, line 191, in saveField
Module Products.ATContentTypes.content.document, line 176, in setText
Module wicked.fieldevent, line 34, in store
Module zope.event, line 23, in notify
Module zope.component.event, line 26, in dispatch
Module zope.component._api, line 130, in subscribers
Module zope.component.registry, line 290, in subscribers
Module zope.interface.adapter, line 535, in subscribers
Module wicked.fieldevent, line 24, in notifyFieldEvent
Module zope.component._api, line 130, in subscribers
Module zope.component.registry, line 290, in subscribers
Module zope.interface.adapter, line 539, in subscribers
Module wicked.fieldevent.meta, line 83, in field_value_set
Module Products.Archetypes.Field, line 1038, in set
Module Products.Archetypes.Field, line 1337, in _process_input
Module Products.Archetypes.BaseUnit, line 203, in setContentType

ValueError: Unknown mime type ['text/html', 'text/html']

Further investigation revealed that the problem did not occur unless the the "Link using UIDs" option was ticked in kupu. Once you had enabled this option in Kupu any attempt to change the body text via an inline edit resulted in you page just sitting there and the above error appearing in the log. Editing the body text via the edit tab worked without any problems. I confirmed this problem on the welcome page of a newly created plone site. The was also replicated by SnowWrite while discussing the issue in Plone IRC. The problem was the same if you created a new page as it was for editing existing content.

It also appears that the problem with inline edits remains if you then go and untick the "Link using UIDs" option in kupu so once you enable that option your inline edits are broken.

This details of my install are:
Zope Version - (Zope 2.10.5-final, python 2.4.4, linux2)
Python Version - 2.4.4 (#2, Oct 4 2007, 22:02:31) [GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)]
plone version: 3.0.6
kupu (Installed product kupu (kupu 1.4.8))

Change History

comment:1 Changed 6 years ago by duncan

  • Owner changed from duncan to ree
  • Component changed from Visual Editor (Kupu) to KSS (Ajax)

It looks like for some reason there are two format fields being submitted when the field is saved. I'd guess that this is a kss problem, so I'm reassigning it as such but bounce it back to me if that doesn't appear to be the case.

comment:2 Changed 6 years ago by ree

  • Cc gotcha, mborche added
  • Status changed from new to assigned

thanks Duncan, I'll have a look.

comment:3 Changed 6 years ago by davby

Same problem here, but I don't have linking by UID enabled. There are two fields named text_text_format (above the visual editor):

<div class="fieldTextFormat">
<label>Textformat</label>
<select id="text_text_format" name="text_text_format" disabled="">
</select>
<input type="hidden" name="text_text_format" value="text/html"/>
</div>

The div, sans the hidden input, appears to be coming from visual.pt in Archetypes (changes to that template are reflected on the page). I have no clue where the hidden input is from though. It is not part of the template in question, so I assume it's added later, but I don't know how to figure out from where.

comment:4 Changed 6 years ago by tomster

  • Cc tomster added

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

FWIW here's a patch you can apply Archetypes/BaseUnit.py as a workaround until ree or gotcha figure out the actual cause:

@@ -198,6 +198,8 @@
         """Set the file mimetype string.
         """
         mtr = getToolByName(instance, 'mimetypes_registry')
+        # temporary hack until http://dev.plone.org/plone/ticket/7893 is resolved:
+        if type(eval(value)) is list: value = eval(value)[0]
         result = mtr.lookup(value)
         if not result:
             raise ValueError('Unknown mime type %s' % value)

comment:6 in reply to: ↑ 5 Changed 6 years ago by tomster

Replying to tomster:

FWIW here's a patch you can apply Archetypes/BaseUnit.py as a workaround until ree or gotcha figure out the actual cause:

@@ -198,6 +198,8 @@
         """Set the file mimetype string.
         """
         mtr = getToolByName(instance, 'mimetypes_registry')
+        # temporary hack until http://dev.plone.org/plone/ticket/7893 is resolved:
+        if type(eval(value)) is list: value = eval(value)[0]
         result = mtr.lookup(value)
         if not result:
             raise ValueError('Unknown mime type %s' % value)

as matthew wilkes quite rightly pointed out, the use of eval for user-supplied data is unsafe! i don't recommend using the patch i supplied above.

a simple:

if value == "['text/html', 'text/html']": value = "text/html" 

does the trick, too.

comment:7 Changed 6 years ago by maurits

  • Cc maurits added

comment:8 Changed 6 years ago by grahamperrin

  • Cc grahamperrin added

comment:9 Changed 6 years ago by hiddenman

  • Cc hiddenman added

comment:10 Changed 6 years ago by davby

  • Cc davby added

comment:11 Changed 6 years ago by duncan

I had a look at this, I can't see how on earth it is happening, but I do know how to stop it being a problem when link-by-uid is enabled. In archetypes/widgets/visual.pt there is:

<div class="fieldTextFormat" tal:condition="python: len(mimetypes) > 1">
    <label i18n:translate="label_text_format">Text Format</label>
    <select>...</select>
</div>

<tal:hidden condition="python: len(mimetypes) == 1">
        <input type="hidden" .../>
</tal:hidden>

Somehow both the select and the input are appearing in the page, but it looks like the two conditions are mutually exclusive. Perhaps something in kss is calling the macro twice in different contexts?

I think I can see why enabling link by uid has an effect: it adds in a transform to the text/x-html-captioned mimetype and really should disable it for text fields but doesn't. If you go to the control panel markup section and make sure you only have one mime type enabled then the select pulldown disappears.

comment:12 Changed 6 years ago by duncan

Sorry, my brain isn't working. The problem isn't that the field appears twice in the HTML: it does that on the edit tab also. The problem is that KSS is sending the value of both fields back to the server. The generated HTML looks like this:

{{{<div class="fieldTextFormat">

<label>Text Format</label>

<select name="text_text_format" id="text_text_format" disabled="">

<option value="text/html" selected="">HTML</option> <option value="text/x-html-captioned">HTML with captioned images</option>

</select>

<input type="hidden" name="text_text_format" value="text/html"/></div> }}} What kupu does is to insert the hidden input and add the 'disabled' attribute to the SELECT. When an HTML form is submitted the values of disabled controls are not sent back to the server: KSS doesn't appear to respect this.

comment:13 Changed 6 years ago by duncan

I've put a workaround into kupu: when it disables the select pulldown it now deletes the name attribute also.

This doesn't stop KSS submitting the field value (in fact it submits the value of every nameless field so according to firebug there's a whole pile of crud gets sent up to the server), but at least Plone manages to ignore the nameless fields.

comment:14 Changed 6 years ago by gotcha

Duncan, thanks for having found our bug !

We were actually submitting values from disabled widgets.

This is fixed on kss.core trunk, branch 1.4 (included in 3.1) and branch 1.2 (included in 3.0)

Duncan, can you check that it does fix this bug and remove the work around ?

comment:15 Changed 6 years ago by ctxlken

I believe this was only patched on kss.core trunk and the 1.4 branch - not the 1.2 branch that ships with Plone 3.0.6.

So, is the recommended resolution to use easy_install to remove (-m) the 1.2 package that comes with plone and then install the 1.4 tagged release candidate (rc1)?

The 1.2 branch doesn't even have the files that were patched on 3/16 as part of this fix, from what I can tell.

Thanks in advance for your advice.

KW

comment:16 Changed 6 years ago by ree

It is safe to run the 1.4 branch of kss.core with Plone 3.0. However I think it is difficult because of egg dependency problems, Products.CMFPlone depends on 1.2 version afaik. I usually leave the dependencies untouched and change the package manually.

If you experience breakage of Plone 3.0 with kss.core 1.4, let me know, even if any problem occurs there is a chance that it's easily fixable.

In addition, - imo - the patch to kss.core is safe and works well, I would apply the patch to 1.2 as well if Godefroid also agrees.

comment:17 Changed 6 years ago by duncan

I put a workaround into kupu 1.4.9 because I wasn't sure whether kss fixes would be available to everyone, so if you don't want to risk upgrading kss just upgrade kupu.

comment:18 Changed 6 years ago by ctxlken

Thanks, Duncan.

I've replaced the kupu 1.4.8 product that shipped with the Plone 3.0.6 bundle with the 1.4.9 branch, and while that seems to have solved the issue with KSS and the text (main body) field, I am still having KSS issues with the Title and Description fields:

1) I can't click into the Title field now to do an in-line edit

2) I can click into the Description field and make an in-line edit, but the Save button doesn't work. If I click Cancel, my edit/change is saved, though (perhaps the button ordering has shifted +1?)

I have the Link using UID and the Allow captioned images options enabled/checked.

So, I'm wondering if I still may need to apply the 1.4 KSS code to my Plone 3.0.6 instance to resolve those last items. I'm not sure what's causing those in-line edit issues. It should be unrelated to my kupu version, as they are not rich fields, of course (so is more likely a KSS-specific thing.)

Can anyone else recreate the issues I have with the Title and Description fields on their 3.0.6 instance?

I noticed that I had the same issue with the Description field when I used kupu 1.4.8, but I did not have the Title field problem.

comment:19 Changed 6 years ago by ctxlken

Please disregard my comment above regarding the Title field. Looks as if we have a custom document_view that is not applying the macro/widget that leverages the KSS.

We are applying it to the Description field, so if anyone can reproduce that issue (Cancel acts as Save, Save does nothing), I'd appreciate hearing back.

comment:20 Changed 6 years ago by ctxlken

I've applied the document_view that ships with Plone 3.0.6 now in order to confirm we don't have any other custom code that is throwing things off.

An update: If I click into the Description field, I need to click Save (it does update the field, but does not remove the save/cancel buttons and the edit box.) I then need to click the Cancel button to remove the buttons and edit box.

If I only click cancel without first clicking save, the edit is not saved.

Again, this is with the default document_view. Anyone else see this behavior?

comment:21 Changed 6 years ago by optilude

Balazs/Godefroid,

Can you confirm whether the fix to KSS is included in Plone 3.1? Kupu 1.4.9 *does* ship with 3.1.

Martin

comment:22 Changed 6 years ago by gotcha

The fix of KSS is included in 3.1 since RC1.

The fix is also in the 1.2 branch of kss.core which would be included if we did a 3.0.7

In the meanwhile, hereunder, the very simple patch to apply to kss.core/kss/core/kukit/forms.js :

--- kukit/kukit.js/branch/1.2/kukit/forms.js	(original)
+++ kukit/kukit.js/branch/1.2/kukit/forms.js	Sun Mar 16 13:06:52 2008
@@ -169,6 +169,9 @@
     // First: update the field in case an editor is lurking
     // in the background
     kukit.fo.fieldUpdateRegistry.doUpdate(element);
+    if (element.disabled) {
+        return null;
+    }
     // Collect the data
     if (element.selectedIndex != undefined) {
         // handle single selects first

comment:23 Changed 6 years ago by russell84

  • Cc russell84 added

comment:24 Changed 6 years ago by davisagli

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

Fixed, as far as I can tell from the discussion here.

Note: See TracTickets for help on using tickets.