testbrowser radio button label lookup error

Reported by Marius Gedminas on 2008-06-28
2
Affects Status Importance Assigned to Milestone
Zope 3
Undecided
Unassigned

Bug Description

Zope 3 renders RadioWidgets in the following way since rev 77293:

  <label for="(id)"><input type="radio" id="(id)"... /> (label text) </label>

This change was made for MSIE compatibility (bug 123507).

Unfortunately, zope.testbrowser (as of rev 87845) gets confused by this and thinks the label for this ItemControl is an empty string.

I'm attaching a patch that adds a failing unit test.

Marius Gedminas (mgedmin) wrote :
Marius Gedminas (mgedmin) wrote :

Sorry, that patch is incomplete. Here's the correct one

Marius Gedminas (mgedmin) wrote :

The problem appears to be in ClientForm. If I invoke ClientForm._show_debug_messages() before that test, I see that the first label (without a for attribute) is processed like this:

    start_label []
    handle_data
    do_input [('type', 'radio'), ...
    handle_data "One"
    end_label

while the second label gets this:

    start_label [('for', 'two')]
    handle_data
    do_input [('type', 'radio'), ..., ('id', 'two')]
    end_label
    handle_data "Two"
    end_label

The end_label function is called too early.

Investigation continues...

Marius Gedminas (mgedmin) wrote :

The _end_label that is called too early is caused by this:

    def start_label(self, attrs):
        ...
        taken = bool(d.get("for"))
        d["__taken"] = taken

and this

    def _add_label(self, d):
        ...
            if self._current_label["__taken"]:
                self.end_label() # be fuzzy

_add_label is called from do_input(). The label gets correctly attached to the input, but the label text remains empty due to the premature end_label() call.

Marius Gedminas (mgedmin) wrote :

This patch for ClientForm 0.2.7 fixes the bug for me.

Marius Gedminas (mgedmin) wrote :

I sent the patch to <email address hidden>, which appears to be the preferred upstream contact address.

Marius Gedminas (mgedmin) wrote :

ClientForm 0.2.8 was released with the bug fix.

I would like to backport the fix to the Zope3.4 branch in svn. Would it be better to update ClientForm.py (currently it's 0.2.2), or just apply the patch?

Marius Gedminas (mgedmin) wrote :

There are complications (aren't there always?).

The KGS (http://download.zope.org/zope3.4/versions.cfg) is pinned to ClientForm 0.2.7. The Zope 3.4 branch in SVN has ClientForm 0.2.2. This is a mismatch.

Ideally, both should be updated to ClientForm 0.2.8. However the "make ClientForm a package and not a module" hack trips upon a new ClientForm bug (__all__ may contain undefined classes) that's not yet fixed in 0.2.8. I've sent the fix for that upstream too, but haven't received a response yet.

I think our options are:

  * Update KGS to 0.2.8, change the 3.4 branch to a copy of 0.2.8 with my bugfix on top of it

  * Wait for upstream 0.2.9, change both KGS and the 3.4 branch to use it.

I really want this bugfix in Zope 3.4.

Changed in zope3:
status: New → In Progress

On Tue, Jul 1, 2008 at 12:13 PM, Marius Gedminas <email address hidden> wrote:
> There are complications (aren't there always?).

> I think our options are:
>
> * Update KGS to 0.2.8, change the 3.4 branch to a copy of 0.2.8 with
> my bugfix on top of it
>
> * Wait for upstream 0.2.9, change both KGS and the 3.4 branch to use
> it.
>
> I really want this bugfix in Zope 3.4.

If you really can't use 3.5, then I'd prefer the second option above.

Perhaps if you explain why 3.5 won't work for you we could come up with
something better.
--
Benji York
Senior Software Engineer
Zope Corporation

Marius Gedminas (mgedmin) wrote :

I'm migrating a large Zope 3.2-based app to Zope 3.4. It's easier if I do it in smaller steps: first make it run with the monolithic tree, then look into eggification. This bug is the only remaining issue that prevents our tests from working on 3.4.

Also, I believe in bugfix releases. Currently in 3.4 zope.testbrowser cannot handle the output that zope.app.form produces. This is partially my fault (for fixing bug 123507), so I feel responsible for the fix.

I'm willing to do the work, but I'm afraid of accidentally stepping on someone's toes, or breaking something out of ignorance.

Benji York (benji) wrote :

On Tue, Jul 1, 2008 at 1:15 PM, Marius Gedminas <email address hidden> wrote:
> I'm migrating a large Zope 3.2-based app to Zope 3.4. It's easier if I
> do it in smaller steps: first make it run with the monolithic tree, then
> look into eggification. This bug is the only remaining issue that
> prevents our tests from working on 3.4.

That's a strong argument for a bug fix release.

> I'm willing to do the work, but I'm afraid of accidentally stepping on
> someone's toes, or breaking something out of ignorance.

The plan you outlined earlier (option 2) seems to be the best way
forward we have. I'm +1 and will help with whatever you need.
--
Benji York
Senior Software Engineer
Zope Corporation

Marius Gedminas (mgedmin) wrote :

ClientForm 0.2.9 is out.

What's the process of updating the KGS?

I do not want to commit the ftest for zope.testbrowser before the KGS is updated, as testbrowser's buildout.cfg uses the KGS and currently wants ClientForm 0.2.7.

I can update ClientForm to 0.2.9 in the Zope 3.4 svn branch right away (all the tests pass).

Changed in zope3:
status: In Progress → Fix Committed
Wolfgang Schnerring (wosc) wrote :

has long been released

Changed in zope3:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers