939523 - Backout or Ammend (comment 3) bug 911876 - Find bar close button is very non-intuitive now

Status: NEW

(Toolkit :: Find Toolbar, defect, P5)

Description

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Compare Findbar's Close Button

For three reasons:

1) To change just the close button of the find toolbar is not the best approach here. The find toolbar's close button should be the same as the other close buttons all around the browser (just open the bookmarks sidebar and compare it's close button). If it is small, then that's a problem with the button layout itself, and not just the find bar's close button specifically.

To have a different close button just in the find bar is just, for lack of a better word, silly in UX terms in my opinion.

2) I agree with bug 911876 comment 20. To just increase the clickable area and not the button itself is very counter-intuitive. No other close button anywhere in the browser behaves like that (again, compare with the sidebar's close button for a quick example) except for the notifications button.

And if you notice carefully, there's a big difference between the find bar and a notification element.

In the notification, the clickable area is tightly limited by an upper border of the nav-bar, the scrollbar/margin on the right, and the content area on the bottom. The same can be said for the find bar of course.

The difference is in the buttons on the left. In the notification, the buttons are always visible, with clearly viewable borders limiting the close button clickable area. In the find bar this is not the case, as the match case button is often unchecked, which makes it seems like the close button clickable area is just "floating around".

It feels weird to click it outside of its visible margins like this.

3) On Linux, the buttons dimensions and margins no longer match the other buttons; see attachment.

I strongly suggest backing out bug 911876. If the size of the close button really is an issue, it should be addressed globally on the button itself (better), or create a specific button for the find bar (worse). Personally, I don't believe the size of the close button is a big issue and shouldn't be touched (best). Has this even been discussed at all? It is after all a major UI change, and if not, it's another big reason to back it out, at least for now.

Comment 1

[Luís Miguel [:quicksaver] (not reading bugmail)]

Mike, I'm sorry to CC you on this without asking. You were assigned bug 776708, so I would appreciate your opinion on this one.

Comment 2

[Mike de Boer [:mikedeboer]]

I asked for feedback about this change in bug 911876.

Comment 3

[Luís Miguel [:quicksaver] (not reading bugmail)]

I realize I should have at least provided some concrete suggestions as to how to proceed. At the least, bug 911876 needs some small fixes if it is to be kept.

IMO in Linux, if the close button resembles the other buttons visually, it should also resemble them in dimensions. I tested this small bit of CSS code in Ubuntu which does the trick, restoring its dimensions and moving it a few pixels away from the margin (it's just very weird to have it stick to the margin like it is now). But of course, I'm not sure if this will have the same effect in other distros:
> .findbar-closebutton {
>   margin-top: 4px;
>   margin-bottom: 4px;
>   -moz-margin-end: 2px;
> }

Also, the patch in bug 911876 misses the fact that in Mac the close button is on the left, not on the right like in the other OS's. Now it has a huge space between the button and the find text field. This bit of CSS should fix that, without distorting it:
> .findbar-closebutton {
>   -moz-margin-start: 0;
>   -moz-padding-start: 4px;
>   -moz-padding-end: 0;
> }

But I'm still of the opinion this could all still be done using negative margins in the close button, without the need to physically move it out of the findbar-container element. I've tested this in Firefox 25 in Windows 7 and 8 in many different DPI and OS themes and in Windows XP. It seems to do the trick very well, it never distorted my find bar and was always within zero-to-one pixels of the find bar's top and bottom border:
> .findbar-closebutton {
>   margin-top: -4px;
>   margin-bottom: -4px;
>   padding-top: 0.6em;
>   padding-bottom: 0.6em;
>   -moz-padding-start: 0;
>   -moz-padding-end: 8px;
> }
> findbar {
>   padding-top: 4px;
>   padding-bottom: 4px;
>   -moz-padding-start: 8px;
>   -moz-padding-end: 0;
> }

And for Mac:
> .findbar-closebutton {
>   margin-top: -4px;
>   margin-bottom: -4px;
>   -moz-margin-start: -2px;
>   -moz-margin-end: 0;
>   padding-top: 6px;
>   padding-bottom: 6px;
>   -moz-padding-start: 6px;
>   -moz-padding-end: 0;
> }

Comment 4

[Luís Miguel [:quicksaver] (not reading bugmail)]

Polite reminder that bug 911876 is on Aurora already, going on Beta soon, and I strongly believe it shouldn't ship in its current state.

Comment 5

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Comparison of Findbar's close button in Linux and OSX

Polite reminder, bug 911876's patch is in beta already.

Added screenshot that shows the difference in the close button in linux and mac osx caused by that bug between firefox 27 (without the bug patch) and nightly (with the bug patch, didn't have beta installed but to my knowledge the findbar layout hasn't changed since), as well as in nightly with my custom CSS code applied, with a slight alteration from what I had proposed in my previous comment.

In linux, I adjusted the height of the button, since it looks like any other toolbar button, it shouldn't be taller than the others. And I kept it a few pixels from the border, it's weird having the button sticking right to the browser margin, but it's not so distant that it should require any "aiming" to hit it, which was the problem addressed by bug 911876:
> .findbar-closebutton {
>   margin-top: 4px;
>   margin-bottom: 4px;
>   -moz-margin-end: 2px;
> }

In Mac the code was assuming the close button was on the right, instead of on the left. I just made it look exactly like it was before, without sacrificing the extra clickable area intended by bug 911876:
> .findbar-closebutton {
>   -moz-margin-start: 0;
>   -moz-padding-start: 6px;
>   -moz-padding-end: 0;
> }

Comment 6

[Adam Tompkins]

Attachment: atdefurchee19704388.txt

Thanks from lipapilvie1983.