Last Comment Bug 591699 - Wrong clickable area for "close tab group" button
: Wrong clickable area for "close tab group" button
Status: VERIFIED FIXED
[fixed-in-fx-team][qa!][testday-20110...
: verified-aurora, verified-beta
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P4 minor
: Firefox 8
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
http://img686.imageshack.us/img686/75...
Depends on: 581748
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-28 17:33 PDT by Please Ignore This Troll (Account Disabled)
Modified: 2016-04-12 14:00 PDT (History)
9 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Explaination of a problem (820.75 KB, image/png)
2010-08-29 01:50 PDT, StyleThing
no flags Details
v1 (5.73 KB, patch)
2010-09-08 19:55 PDT, Sean Dunn
no flags Details | Diff | Review
v1 (the real v1) (529 bytes, patch)
2010-09-08 19:58 PDT, Sean Dunn
dietrich: review+
ian: feedback+
mbeltzner: approval2.0-
Details | Diff | Review
v2 (2.89 KB, patch)
2011-08-08 00:24 PDT, Raymond Lee [:raymondlee]
ttaubert: review+
Details | Diff | Review
Patch for checkin (2.98 KB, patch)
2011-08-14 23:14 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review

Description Please Ignore This Troll (Account Disabled) 2010-08-28 17:33:15 PDT
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 2010-08-29 01:50:46 PDT
Created attachment 470269 [details]
Explaination of a problem

So we need a small change in tabview.css file
Comment 2 Ian Gilman [:iangilman] 2010-09-07 23:34:37 PDT
looks like this should be a Windows-specific fix
Comment 3 Sean Dunn 2010-09-08 19:55:57 PDT
Created attachment 473366 [details] [diff] [review]
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 2010-09-08 19:58:30 PDT
Created attachment 473368 [details] [diff] [review]
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) 2010-09-12 15:27:35 PDT
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 2010-09-12 16:18:45 PDT
(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) 2010-09-12 16:36:55 PDT
(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] 2010-09-13 10:15:57 PDT
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) 2010-09-13 17:05:59 PDT
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) 2010-09-14 01:48:50 PDT
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) 2010-09-14 03:15:13 PDT
(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] 2010-09-14 06:13:32 PDT
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) 2010-09-14 06:32:55 PDT
(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] 2010-09-14 10:33:39 PDT
(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] 2010-09-16 16:50:21 PDT
* 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] 2010-09-17 10:34:11 PDT
(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 2011-01-10 10:07:33 PST
bugspam (moving b9 to b10)
Comment 18 Kevin Hanes 2011-01-10 10:10:20 PST
bugspam (removing b9)
Comment 19 Michael Yoshitaka Erlewine [:mitcho] 2011-01-20 16:35:50 PST
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 2011-01-21 14:11:23 PST
bugspam. Moving b10 to b11
Comment 21 Kevin Hanes 2011-01-21 14:12:57 PST
bugspam. Removing b10
Comment 22 Michael Yoshitaka Erlewine [:mitcho] 2011-02-09 22:55:44 PST
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) 2011-02-10 05:04:55 PST
It is still an actual issue. And the patch is great, but never got reviewed.
Comment 24 Michael Yoshitaka Erlewine [:mitcho] 2011-02-14 10:07:48 PST
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] 2011-02-23 10:50:23 PST
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) 2011-03-04 18:26:54 PST
the bug is fixed, why is it still reopened?
Comment 27 Kevin Hanes 2011-03-31 10:51:43 PDT
bugspam
Comment 28 Tim Taubert [:ttaubert] 2011-05-27 02:22:30 PDT
bugspam
Comment 29 Tim Taubert [:ttaubert] 2011-05-27 02:27:26 PDT
bugspam
Comment 30 Tim Taubert [:ttaubert] 2011-07-24 18:38:23 PDT
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] 2011-08-08 00:24:50 PDT
Created attachment 551396 [details] [diff] [review]
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] 2011-08-08 05:02:15 PDT
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] 2011-08-14 14:23:32 PDT
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] 2011-08-14 23:14:28 PDT
Created attachment 553117 [details] [diff] [review]
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] 2011-08-15 07:09:53 PDT
http://hg.mozilla.org/integration/fx-team/rev/c762e01dd6dc
Comment 36 Rob Campbell [:rc] (:robcee) 2011-08-16 08:41:01 PDT
http://hg.mozilla.org/mozilla-central/rev/c762e01dd6dc
Comment 37 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 15:50:03 PDT
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] 2011-10-01 02:12:37 PDT
(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] 2011-10-05 05:28:38 PDT
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

Note You need to log in before you can comment on or make changes to this bug.