282540 - Search box in show/edit cookie sheets should have "looking glass" icon

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

There's been some discussion on the Camino MozillaZine forum that describes the
cookie search field as "a nondescript text field," and for those who haven't
been following the development of these sheets and the cool new search ability,
it's not immediately obvious what the field does.

Both the toolbar search field and the search field in the Bookmarks Mgr have the
"looking glass" icon, so that should be sufficient for clarifying the purpose of
the field in these sheets.

CC'ing olivier who did the original work on the field and jasper the UI guru :)

Comment 1

[olivier]

make sense, i'll try to update this.

Comment 2

[olivier]

Attachment: patch file

modified the search box so that it display a magnifying glass even when no pop
up is present. It now display a magnifying glass without the frop down triangle
in that case.

Comment 3

[olivier]

Attachment: new image for search box without drop down

the image should be added to the project in /resources/images/chrome/ and added
to the xcode file

Comment 4

[Jasper]

We really need to look into creating some sort of custom service for all our
version of the search field. Especially because we want this version but also a
small version for in the bookmark manager. Isn't there someway to now start
using the round text field as that's supported on 10.2+?

Comment 5

[olivier]

(In reply to comment #4)
> We really need to look into creating some sort of custom service for all our
> version of the search field. Especially because we want this version but also a
> small version for in the bookmark manager. Isn't there someway to now start
> using the round text field as that's supported on 10.2+?

I thought their was only one search field class in Camino. It would be nice to
use the system one, that would make the whole thing easier. It all depend on
what are the minimum requirement for camino. If 10.2 is the minimum, then we can
just use the system one.

Comment 6

[Jasper]

Well as of camino 0.9 we will support 10.2 and higher, so I think it's a good
idea  to update our own field class to using the round field provided by the
system and only using our own spyglass and cancel images. Taking into account
the fact that we will want to use a small version aswell.

Comment 7

[olivier]

(In reply to comment #6)
Why not just drop the custom one and use the system one directly? I think the
one from the system provides all that is needed.

Comment 8

[Jasper]

That would be good to as long as you can guarantee it also works on 10.2? This
would then mean we'd have to change all other parts of our code that use our own
search class.

Comment 9

[olivier]

Just checked and the round field is 10.2 and higher, but the NSSearchTextField
is 10.3 and higher.
I will look into modifying our search field to us the build in round cell.

Comment 10

[olivier]

Attachment: new search box using the build in round text field

The search box now display two distinct magnifying glass depending on weither
there is a pop up or not. it also uses the OS rounded text field.
The image to build the search box can be removed from the project (searchLeft,
searchMiddle, searchRight).

Comment 11

[olivier]

Comment on attachment 175006 [details] [diff] [review]
new search box using the build in round text field

The searchPopupLess image is still needed

Comment 12

[Bruce Davidson]

Comment on attachment 175006 [details] [diff] [review]
new search box using the build in round text field

[Reviewing for Ludo]

>   }
>+  else
>+  {
>+    [_popUpLessImage compositeToPoint:_popUpImageOrigin operation:NSCompositeSourceOver];
>+  }

Nit: Follow the "drop all braces on single line dependent statements" style.

Your pop-up-less magnifying glass image is the same width as the one with the
pop-up disclosure arrow. This means there's a lot of whitespace before the
text; the text appears rather "adrift" in the box. Consider cropping the image
a little on the right.

Other than that it looks fine. Recommend Ludo r+s with those nits.

Comment 13

[olivier]

Attachment: patch including code formatting comments

I did not move the text when there is no popup, because that is the way apple
does it. The text is a bit far to the right, but this way it does not move.
Look in the xCode toolbar.

Comment 14

[olivier]

Attachment: new patch file with text more to the left

i moved the text five pizel to the left both when the popup is present or not
as it seem to be what apple has done in theirs.

Comment 15

[Mike Pinkerton (not reading bugmail)]

make sure this works correctly on 10.2. I did some mods to this file for another
project and had redraw issues with the text cell only on jaguar.

Comment 16

[olivier]

I don't have jaguar around. Could somebody check this on jaguar?

Comment 17

[Jasper]

Attachment: image to show 10.2 bug

YUCK thank you apple for this "wonderfull" bug.

K so look at the image and you will see the infamous "top doesn't draw
correctly" issue Mike rightfully pointed out.

Comment 18

[Ludovic Hirlimann [:Usul]]

Oliver I can't build at the momment so ask review to someone else or wait for a
week or two And I should get my powerbook back.

Comment 19

[olivier]

Attachment: patch file using the custom build round search box

Comment 20

[Mike Pinkerton (not reading bugmail)]

looks reasonable, though 'searchpopupless' is a terrible terrible name ;)

Comment 21

[Mike Pinkerton (not reading bugmail)]

oh, fwiw, i sent a code sample to DTS (ready to use an incident) asking about
using a custom cell with the rounded text bezel on 10.2. Their response was
"sorry, you can't do this well/correctly on 10.2, use images to draw the search
field if you want to use a custom cell". 

good times ;)

Comment 22

[Mike Pinkerton (not reading bugmail)]

landed, renamed icon/member ;). thanks!

Comment 23

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 176820 [details] [diff] [review]
patch file using the custom build round search box

r=pink