Closed Bug 344379 Opened 18 years ago Closed 18 years ago

label click doesn't focus select/select1 properly

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: fixed1.8.0.8, fixed1.8.1.1)

Attachments

(3 files, 2 obsolete files)

When I click on label of select/select1 control then they are not focused. It's right for all select/select1 controls exceptin select[@apparance='minimal'] for xhtml.
Attached file testcase (obsolete) —
Attached file html testcase
Assignee: xforms → surkov.alexander
Attachment #228969 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attached file xul testcase
Attached patch patch (obsolete) — Splinter Review
Attachment #232512 - Flags: review?(Olli.Pettay)
Comment on attachment 232512 [details] [diff] [review]
patch


>+      <method name="isItemElement">
>+        <parameter name="aElement"/>
>+        <body>
>+          return aElement.getAttribute("anonid", "wcontrol");
>+        </body>
>+      </method>

Fix this to be like xhtml version

>+      <!-- Advance focus to native widget.
>+        The method assuemes native widget has 'focus' method. If it is not so
>+        then underlying widget uses 'controlwidget-xformsfocus-base' binding
>+        instead.

Check spelling

>+  <!-- The 'controlwidget-xformsfocus-base' is the base binding of underlying
>+    controls. The widget implements 'focus' method to advance a focus to
>+    native widget of underlying control.

'to advance a focus'
Is that good English?

I'm not too happy to add again a new binding, things are getting complex.
But if you think this needed, then ok to me.

We really should have a graph of bindings somewhere.
Hmm, perhaps I could write a tool for that... let's see.
Something which automatically generates a tree of xforms xbl bindings...
Attachment #232512 - Flags: review?(Olli.Pettay) → review+
Attached patch patch2Splinter Review
> I'm not too happy to add again a new binding, things are getting complex.
> But if you think this needed, then ok to me.

I don't like it a lot too. I just can't see more nice approach.

> We really should have a graph of bindings somewhere.
> Hmm, perhaps I could write a tool for that... let's see.
> Something which automatically generates a tree of xforms xbl bindings...
> 

That's good idea.
Attachment #232512 - Attachment is obsolete: true
Attachment #232933 - Flags: review?(aaronr)
Comment on attachment 232933 [details] [diff] [review]
patch2

the patch looks good.  But I'm r-'ing because the patch doesn't always work.  For example, in your xhtml testcase, if I click on the select1 @appearance='full' and hit the up arrow, it moves the radio button selection.  But then the focus seems to go to the document and no matter how many times I click back on that label, the focus won't go to the radio button again.  JS errors show up in the console.

Even easier to break the xul testcase.  For example, clicking on the select @appearance='compact' label, focus doesn't appear in the list box.  I hit the arrow once and the first item gets selected but if I hit the down arrow to select the next item down, it doesn't work.  More JS errors in the console.
Attachment #232933 - Flags: review?(aaronr) → review-
(In reply to comment #7)
> (From update of attachment 232933 [details] [diff] [review] [edit])
> the patch looks good.  But I'm r-'ing because the patch doesn't always work. 
> For example, in your xhtml testcase, if I click on the select1
> @appearance='full' and hit the up arrow, it moves the radio button selection. 
> But then the focus seems to go to the document and no matter how many times I
> click back on that label, the focus won't go to the radio button again.  JS
> errors show up in the console.
> 
> Even easier to break the xul testcase.  For example, clicking on the select
> @appearance='compact' label, focus doesn't appear in the list box.  I hit the
> arrow once and the first item gets selected but if I hit the down arrow to
> select the next item down, it doesn't work.  More JS errors in the console.
> 

This bug is about providing interface to advance focus to native widget for select, since previsouly that didn't work properly. I belive issues you pointed me is not of this bug actually.

1) "Focus is lost when I move through html select-full/xul select-compact" problem is we rebuild select control and focus is lost. We have the problem on trunk.
2) "Focus doesn't appear in XUL select-compact" problem is how focus works for xul:listbox in our case.

Let me file new bugs for these issues since I guess we don't deal with regression here.
So, Aaron, what is your verdict?
(In reply to comment #9)
> So, Aaron, what is your verdict?
> 

The patch looked fine, so if you are going to fix these behaviors under a different bug, that is fine.  r=me
Attachment #232933 - Flags: review- → review+
(In reply to comment #10)
> (In reply to comment #9)
> > So, Aaron, what is your verdict?
> > 
> 
> The patch looked fine, so if you are going to fix these behaviors under a
> different bug, that is fine.  r=me
> 

These bugs are bug 339827 and bug 349461.
checked into trunk for surkov
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
checked into 1.8.0 branch on 2006/09/21
Keywords: fixed1.8.0.8
checked into 1.8 branch on 2006/11/21
Keywords: fixed1.8.1.1
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: