591699 - Wrong clickable area for "close tab group" button

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P4)

Description

[Please Ignore This Troll (Account Disabled)]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre

"Close tab group" button looks like a regular cross, but it's clickable area is moved right for 6-9 px, look at the gif video.

Reproducible: Always

Steps to Reproduce:
1. Open TabCandy window (ctrl+space)
2. Try to aim the area around the "Close tab group" button.
Actual Results:  
The borders of this area a more to right, than needed.

Expected Results:  
Move it a little bit to the left, to fit the borders of the button's image.

GIF video.
http://img686.imageshack.us/img686/7556/98929373.gif

Comment 1

[StyleThing]

Attachment: Explaination of a problem

So we need a small change in tabview.css file

Comment 2

[Ian Gilman [:iangilman]]

looks like this should be a Windows-specific fix

Comment 3

[Sean Dunn]

Attachment: v1

Following StyleThing's recommendation, I added the background-position: 50% 50%; line, which moves the image to the center of the click box for the winstripe theme.

Comment 4

[Sean Dunn]

Attachment: v1 (the real v1)

Following StyleThing's recommendation, I added the background-position: 50%
50%; line, which moves the image to the center of the click box for the
winstripe theme.

Comment 5

[Please Ignore This Troll (Account Disabled)]

The space at right from the cross is still clickable, though I think it's ok, since it makes easier to point to that button.

Comment 6

[Sean Dunn]

(In reply to comment #5)
> The space at right from the cross is still clickable, though I think it's ok,
> since it makes easier to point to that button.

I had understood the problem as not being that the clickable area was too big, but that the image itself wasn't centered properly inside of it.

Comment 7

[Please Ignore This Troll (Account Disabled)]

(In reply to comment #6)

well, it is bigger than the cross, and either it needs to be fixed or make it even bigger, to convert bug into feature: since tab groups can be resized only via dragging by the bottom right corner - it would be a nice feature to make the whole top right corner be clickable, so the user doesn't need to point the cross accurately.

Comment 8

[Ian Gilman [:iangilman]]

Comment on attachment 473368 [details] [diff] [review]
v1 (the real v1)

I agree we shouldn't make the clickable area smaller, or it will impact usability. There's a hover effect, so I don't think people are going to be confused about what's going to get clicked.

Comment 9

[Please Ignore This Troll (Account Disabled)]

well(In reply to comment #8)
> I agree we shouldn't make the clickable area smaller

then maybe make it bigger, to make aiming that button much easier.
As I already said - since we can't resize the tab group by dragging top right corner - we can occupy it for our needs.

Comment 10

[Dietrich Ayala (:dietrich)]

Comment on attachment 473368 [details] [diff] [review]
v1 (the real v1)

r+a=me. please confirm drugoy's comment. if it makes sense to enlarge the area, let's do it here.

Comment 11

[Please Ignore This Troll (Account Disabled)]

(In reply to comment #10)
> Comment on attachment 473368 [details] [diff] [review]
> v1 (the real v1)
> 
> r+a=me. please confirm drugoy's comment. if it makes sense to enlarge the area,
> let's do it here.

confirm how? open tabsets yourself and just try to resize a group anyhow. The only way to do it is to drag the _bottom_ right corner. Nothing happens if you drag other corners.

Comment 12

[Michael Yoshitaka Erlewine [:mitcho]]

I personally don't think the clickable area should be larger than the area which triggers the :hover.

In other news, not sure why this bug was closed and then reopened... the patch has not yet been committed, right? Sean, you'll want to do this using checkin-needed. Hit us up on IRC if you want help with that. http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Comment 13

[Please Ignore This Troll (Account Disabled)]

(In reply to comment #12)
> I personally don't think the clickable area should be larger than the area
> which triggers the :hover.

than make the are which triggers the :hover bigger. The point is to make aiming the cross button easier. By enlarging this area up till borders at right and on top of the cross.

Comment 14

[Ian Gilman [:iangilman]]

(In reply to comment #13)
> (In reply to comment #12)
> > I personally don't think the clickable area should be larger than the area
> > which triggers the :hover.
> 
> than make the are which triggers the :hover bigger. The point is to make aiming
> the cross button easier. By enlarging this area up till borders at right and on
> top of the cross.

I'm certainly down with making the click area larger if need be. Looks like it's currently 16 pixels, which is likely the same size it is on the Mac. 

I leave it to Aza; what say you? Should we make it larger? If so, how big should it be, and should we do the same for other platforms?

Comment 15

[Aza Raskin [:aza]]

* Let's make the clickable target go to the corner.
* Because the :hover already shows the the button activates, I do not believe we need to change the graphic to be larger.

Comment 16

[Ian Gilman [:iangilman]]

(In reply to comment #15)
> * Let's make the clickable target go to the corner.
> * Because the :hover already shows the the button activates, I do not believe
> we need to change the graphic to be larger.

Should the clickable target extend to the corner on the other platforms as well? I assume so.

Comment 17

[Kevin Hanes]

bugspam (moving b9 to b10)

Comment 18

[Kevin Hanes]

bugspam (removing b9)

Comment 19

[Michael Yoshitaka Erlewine [:mitcho]]

I'm confused... did this land and then get reopened? Or did it never land?

Sean, if it's the latter, I assume it's patch rotted. If we still want this, please unrot for landing.

Comment 20

[Kevin Hanes]

bugspam. Moving b10 to b11

Comment 21

[Kevin Hanes]

bugspam. Removing b10

Comment 22

[Michael Yoshitaka Erlewine [:mitcho]]

Is this still an issue? What ever happened to this bug?

[bugspam: mitcho's late-night betaN triage; feel free to comment or reverse]

Comment 23

[Please Ignore This Troll (Account Disabled)]

It is still an actual issue. And the patch is great, but never got reviewed.

Comment 24

[Michael Yoshitaka Erlewine [:mitcho]]

Comments 15 and 16 have not been adressed. Will take this bug and try to create a patch that adresses them.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 473368 [details] [diff] [review]
v1 (the real v1)

This is no longer approved as it's impossible to tell if it's still wanted. Renominate if it is.

Comment 26

[*I need to change my userid so it's less racist)]

the bug is fixed, why is it still reopened?

Comment 27

[Kevin Hanes]

bugspam

Comment 28

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 29

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 30

[Tim Taubert [:ttaubert] (inactive)]

bugspam

(Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)

Comment 31

[Raymond Lee [:raymondlee]]

Attachment: v2

Addressed comment 15 and comment 16.  The clickable area goes to the top corn, the same for the :hover state.

Comment 32

[Raymond Lee [:raymondlee]]

Comment on attachment 551396 [details] [diff] [review]
v2

Passed Try with oranges not related to this patch.
http://tbpl.mozilla.org/?tree=Try&rev=1c23547c3ca5

Comment 33

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 551396 [details] [diff] [review]
v2

Review of attachment 551396 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

r=me with the little fix.

::: browser/themes/gnomestripe/browser/tabview/tabview.css
@@ +220,5 @@
>    z-index: 10;
> +  top: 0px;
> +  right: 0px;
> +  padding-top: 6px;
> +  padding-right: 6px;

Please use "width: 22px" and and "height: 22px" instead of the padding here so this is a bit cleaner and you don't need to touch padding values with [dir=rtl]. (same with the other themes)

Comment 34

[Raymond Lee [:raymondlee]]

Attachment: Patch for checkin

(In reply to Tim Taubert [:ttaubert] from comment #33)
> Please use "width: 22px" and and "height: 22px" instead of the padding here
> so this is a bit cleaner and you don't need to touch padding values with
> [dir=rtl]. (same with the other themes)

Fixed.

Comment 35

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/fx-team/rev/c762e01dd6dc

Comment 36

[Rob Campbell [:rc] (:robcee)]

http://hg.mozilla.org/mozilla-central/rev/c762e01dd6dc

Comment 37

[u279076]

Tested on Firefox 8.0b1:

While the left and bottom bounds seem to be that of the visible bounds of the button, the top and right bounds appear to be the top and right bounds of the tab group itself. In other words, I can put my mouse cursor well above or to the right of the button. In fact, clicking the absolute top-right corner of the tab group still clicks the button.

Is this expected?

Comment 38

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #37)
> While the left and bottom bounds seem to be that of the visible bounds of
> the button, the top and right bounds appear to be the top and right bounds
> of the tab group itself. In other words, I can put my mouse cursor well
> above or to the right of the button. In fact, clicking the absolute
> top-right corner of the tab group still clicks the button.
> 
> Is this expected?

Yes, it is. Please see comment #15 and comment #16. Thanks for verifying :)

Comment 39

[Vlad [QA]]

I have also verified this and I got the same behavior as Anthony describes in comment37. Based also on comment38, setting resolution to Verified Fixed

Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:9.0a2) Gecko/20111004 Firefox/9.0a2
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111004 Firefox/10.0a1

Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 5.1; rv:9.0a2) Gecko/20111004 Firefox/9.0a2
Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111004 Firefox/10.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a2) Gecko/20111004 Firefox/9.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111004 Firefox/10.0a1

Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111003 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111004 Firefox/10.0a1