414842 - Unified Back-/Forward button isn't Fittsy (anymore)

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Simon Bünzli]

StR:
1. Maximize your window
2. Visit two bookmarks in the same tab
3. Click at the left screen border at the height of the Back button

Expected result:
We navigate back.

Actual result:
The click was in vain (there's now a 2px margin).

The same happens with the dropmarker when the button is moved to the very right of the screen. This is even trickier to spot, as there's currently no hover feedback for the dropmarker.

Comment 1

[William R. "Xer" Cope]

The offending code is at line 261 of browser.css:
toolbar[iconsize="large"][mode="icons"] #unified-back-forward-button {
  -moz-padding-start: 2px;
  -moz-padding-end: 2px;
}

It's easily fixed without any visual changes by switching that with:
toolbar[iconsize="large"][mode="icons"] #unified-back-forward-button > #back-button {
  -moz-padding-start: 2px;
}
toolbar[iconsize="large"][mode="icons"] #unified-back-forward-button > #back-forward-dropmarker {
  -moz-padding-end: 2px;
}

Comment 2

[Simon Bünzli]

Attachment: patch per comment #1

Comment 3

[Simon Bünzli]

Are you up for your first bug, William?

Comment 4

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

This does not block the final release of Firefox 3.

Comment 6

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

Comment on attachment 301062 [details] [diff] [review]
patch per comment #1

a=beltzner for 1.9

Comment 7

[Reed Loden [:reed]]

Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.171; previous revision: 1.170
done

Comment 9

[mcdavis941 (sporadically reading bugmail)]

Looks like the Fittsy-ness has fallen off again.

Should be reopened, or a new bug filed.  

I see how to fix it, but I don't want to swoop on XerBlade's bug.  William, do you want this?

Comment 10

[mcdavis941 (sporadically reading bugmail)]

s/Fittsy-ness/Fittsy version of the padding/

Comment 11

[chinmay]

IMHO, the problem is not with the padding, but with the shape of the active (target) area itself. The button may *look* round, but that is no reason to not extend the clickable area to the entire rectangular area on the left, especially since back button is the left-most on screen.

Essentially, something like below: (Silly ASCII art :) May not work on your screen!)

+------*
|click- *
|able   *
+------*

Comment 12

[mcdavis941 (sporadically reading bugmail)]

ASCII art FTW! :) Agree that the target area should extend to the left edge.  The patch that landed for this bug did just what you're suggesting, but because of other changes its CSS selectors no longer have precedence and don't apply the padding any more.

Comment 13

[William R. "Xer" Cope]

WFM in the current nightly. Are you using hourlies?
I guess I'll see either way once the next nightly lands.

Comment 14

[Dão Gottwald [:dao]]

(In reply to comment #12)
> because of other changes its CSS selectors no longer have precedence

which changes?

Comment 15

[mcdavis941 (sporadically reading bugmail)]

(In reply to comment #13)
> WFM in the current nightly. Are you using hourlies?

William, I'm looking at the 02-26 nightly.

(In reply to comment #14)
> (In reply to comment #12)
> > because of other changes its CSS selectors no longer have precedence
> 
> which changes?

Dao, basically, with the 02-26 nightly, this rule from the patch for this bug:

 toolbar[iconsize="large"][mode="icons"] #back-button {
   -moz-image-region: rect(0px 394px 34px 360px);
+  -moz-padding-start: 2px;
 }

is setting padding which is overruled by the padding set by this rule:

toolbar[iconsize="large"][mode="icons"] 
  #unified-back-forward-button > .toolbarbutton-1 {
  -moz-appearance: none;
  border: none;
  padding: 0;
}

The back button in the unified case ends up with no padding on the left.  Still Fittsy though.

Comment 16

[mcdavis941 (sporadically reading bugmail)]

(In reply to comment #14)
> (In reply to comment #12)
> > because of other changes its CSS selectors no longer have precedence
> 
> which changes?
> 
And I probably should have said "other rules" instead of "other changes".  Sometimes I really wish Bugzilla had an edit post function ...

Comment 17

[William R. "Xer" Cope]

Still WFM. With 02-27 nightly as well. I thought -moz-padding-start was supposed to override simple padding in normal situations? Seems to be doing that here anyway.

Just to be safe, maybe should change "toolbar[iconsize="large"][mode="icons"] #back-button" to "toolbar[iconsize="large"][mode="icons"] #unified-back-forward-button > #back-button" (like my original fix, not the actual patch zeniko made from it, already had anyway) just to be sure the selector always defeats that other one in importance (ID has greater importance than class where selectors are otherwise equal, last I checked), so we always get the right padding (even in weird/ambiguous cases)?

Either way, I don't think this bug should be reopened to fix what is essentially a new bug caused by a different thing (even if the problem caused by the bug is essentially the same). If you want to open a new bug, I don't see a reason not to, just that I'm not seeing the bug myself. If you do, post it in a comment here, and, if I'm around (I work so much lately right now is about the only time I can get on) I might go ahead and throw up a patch based on the fix I just suggested, or someone else can if they want (don't really care). You or someone else who actually sees the bug can then test it (no point in me testing it, since I don't see the bug to begin with...), and we might see about getting that checked in for... RC1 or whatever's next (I haven't checked the roadmap in a long time).