Bug 3852 – typeahead doesn't work in multiple row select boxes.

• Component: Forms
• Status: RESOLVED FIXED

[Joost de Valk (AlthA)]

see forthcoming attachment.

[Joost de Valk (AlthA)]

Created attachment 2790 [details]
testcase

[Darin Adler]

I imagine that Dave get's around to doing the non-NSView version of this control he'll fix this.

[Rosyna]

Created attachment 4036 [details]
Patch to enable type select on List Boxes

This patch uses the new UnicodeTypeSelect API in 10.4 and later (was SPI and
had different symbol names previous to that).

[Rosyna]

review: ?

[Dave Hyatt]

Comment on attachment 4036 [details]
Patch to enable type select on List Boxes

r=me

[Darin Adler]

UCTypeSelectReleaseSelector needs to be called in finalize as well as dealloc.

[Darin Adler]

Comment on attachment 4036 [details]
Patch to enable type select on List Boxes

This patch is great, but I think it still needs a little work:

I don't see any reason for the check for a refcon that's nil -- can't happen.
The check for a box of nil is required of course. But the coding style is not
quite right. The * is not supposed to be next to the type name (see the coding
style document on our website). Also I don't think the extra parentheses around
box->count() make things clearer.

Instead of calling getNSString() and casting to CFStringRef this should be
calling getCFString().

Also, if outString is 0, then we should save code by not calling itemAtIndex at
all (put that code inside the if).

The brace should be on the same line as the if. We use 0 in our code, not NULL
(see coding style document).

There's no need to use "UPP" in the code at all. This code is only used in
Mach-O and so both NewIndexToUCStringUPP and DisposeIndexToUCStringUPP are
no-ops and should be left out entirely. The code can be used as is.

There's no need to cast self to (void*) -- the cast should be left out.

The extra space before "byExtendingSelection" should be omitted.

Even though these are small things, I'm changing the review+ to review- so we
can get them fixed before landing the patch.

[Rosyna]

Created attachment 4170 [details]
New Patch to enable type select on List Boxes

Added changes as specified by Darin. Now uses ICU to determine if the character
is in the range of graphic characters as defined by D13a of the Unicode 4.0
standard.

[Rosyna]

Created attachment 4172 [details]
Anothr testcase with ß as an item

Added a new testcase to check against option S.

[Rosyna]

Created attachment 4201 [details]
Newer patch to enable typeselect on list boxes

Bah, I kind of suck. Had two checks for outString. Only one is needed.
outString shall never be false much like item shall never be false.

[Rosyna]

Created attachment 4203 [details]
Newest typeselect list box patch

Sigh, the old one was the wrong one. Apologies to the people that get emailed
bug reports.

[Darin Adler]

Comment on attachment 4203 [details]
Newest typeselect list box patch

Looks good. There are a couple "=" and "==" in there without spaces around
them. And a "{" without a space before it. But everything else looks great.

r=me

[Eric Seidel (no email)]

Comment on attachment 4203 [details]
Newest typeselect list box patch

Darin and I discussed.	In order to make this easier on the lander (who may not
be darin), I would like to ask you to first fix these syle issues, so tha
landing is just as simple as cvs-applying the patch.

[Rosyna]

Created attachment 4243 [details]
More Newest Patch to enable type select on List Boxes

???????
Addresses the syle issues. I think I got them all. I'm not as sensitive to
spaces as some others.

[Darin Adler]

Comment on attachment 4243 [details]
More Newest Patch to enable type select on List Boxes

r=me