278881 - select elements aren't incremental by default

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

According to the spec, http://www.w3.org/TR/xforms/index-all.html#ui-selectMany,
xforms:select elements should default to being incremental="true".  This means
that that xforms-value-changed events are sent out for every item that is
clicked on, rather than just one being sent out when the control loses focus.

I will attach a testcase.

Comment 1

[aaronr]

Attachment: testcase that shows problem

For every item in the select element that is clicked on, the input field next
to it should be updated.  Right now if the value changes, it won't be reflected
in the input field until the select element loses focus.

Comment 2

[aaronr]

Attachment: proposed fix

pretty simple fix.  If incremental attribute value is "true" or empty (meaning
there is no incremental attribute on the element), then we'll have incremental
xforms:select elements.

Comment 3

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Could you update the patch? I think it contains code for some other bug (or
is it for branch).

Comment 4

[aaronr]

Attachment: patch from trunk

Sorry, I thought that this was the only patch that I had in that file.	This is
definitely the right patch now.

Comment 5

[aaronr]

Comment on attachment 172014 [details] [diff] [review]
proposed fix

old patch, been replaced.

Comment 6

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Comment on attachment 172288 [details] [diff] [review]
patch from trunk


>-  PRBool isIncremental = value.EqualsLiteral("true");
>+
>+  // the default incremental value for a select element is 'true' according
>+  //   to the spec, so if there is no incremental value, assume true.
Fix this indentation.

>+  PRBool isIncremental = value.EqualsLiteral("true") || value.IsEmpty();
Why not 
PRBool isIncremental = !value.EqualsLiteral("false");

With those r=me

Comment 7

[aaronr]

Comment on attachment 172288 [details] [diff] [review]
patch from trunk

updating with new patch for smaug's comments

Comment 8

[aaronr]

Attachment: updated patch

I don't seen an indentation problem in the patch or when the patch it is
applied to my tree.

changed test to smaug's suggestion.  I was thinking that we should handle all
non true/false attribute values as false, but I guess that the proper way is to
handle them as if the attribute weren't there at all, and thus we need to
behave the same way as the default which is incremental='true' behavior.  My
bad.

Comment 9

[aaronr]

Attachment: fixed indentation

ok, changed indentation.  I guess that you meant that they didn't line up
perfectly.  I guess I prefer a reverse-paragraph style.  The comment note that
bugzilla generated showed a highly exaagerated indentation (like 8 char's) so I
thought that is what you meant...an exaageration that I didn't see in the real
patch.

Comment 10

[Allan Beaufour]

Removed the Windows linebreaks and checked in.