testbrowser radio button label lookup error

Bug #243785 reported by Marius Gedminas
2
Affects Status Importance Assigned to Milestone
Zope 3
Fix Released
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.

Revision history for this message
Marius Gedminas (mgedmin) wrote :
Revision history for this message
Marius Gedminas (mgedmin) wrote :

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

Revision history for this message
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...

Revision history for this message
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.

Revision history for this message
Marius Gedminas (mgedmin) wrote :

This patch for ClientForm 0.2.7 fixes the bug for me.

Revision history for this message
Marius Gedminas (mgedmin) wrote :

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

Revision history for this message
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?

Revision history for this message
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
Revision history for this message
Benji York (benji) wrote : Re: [Bug 243785] Re: testbrowser radio button label lookup error

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

Revision history for this message
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.

Revision history for this message
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

Revision history for this message
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
Revision history for this message
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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.