Closed Bug 414842 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox :: Theme, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3 beta4

People

(Reporter: zeniko, Assigned: XerBlade)

References

Details

(Keywords: access, regression)

Attachments

(1 file)

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.
Flags: blocking-firefox3?
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;
}
Attachment #301062 - Flags: review?(gavin.sharp)
Are you up for your first bug, William?
Assignee: nobody → XerBlade
Blocks: 386228
Blocks: 411725
No longer blocks: 386228
This does not block the final release of Firefox 3.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Duplicate of this bug: 417513
Attachment #301062 - Flags: review?(gavin.sharp) → review+
Target Milestone: --- → Firefox 3 beta4
Comment on attachment 301062 [details] [diff] [review]
patch per comment #1

a=beltzner for 1.9
Attachment #301062 - Flags: approval1.9? → approval1.9+
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
Status: NEW → RESOLVED
Closed: 12 years ago
Component: Toolbars → Theme
Keywords: checkin-needed
QA Contact: toolbars → theme
Resolution: --- → FIXED
Duplicate of this bug: 418103
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?

s/Fittsy-ness/Fittsy version of the padding/
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   *
+------*

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.
WFM in the current nightly. Are you using hourlies?
I guess I'll see either way once the next nightly lands.
(In reply to comment #12)
> because of other changes its CSS selectors no longer have precedence

which changes?
(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.



(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 ...
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).
You need to log in before you can comment on or make changes to this bug.