Closed Bug 571454 Opened 14 years ago Closed 13 years ago

Back button does not work along edge of screen (would like to see Fitts Law applied when on left most edge of the toolbars)

Categories

(Firefox :: Theme, defect)

All
Windows XP
defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 8

People

(Reporter: kurt_dixon150, Assigned: jaws)

References

Details

(Keywords: verified-beta, Whiteboard: [qa!][testday-20110930])

Attachments

(3 files, 9 obsolete files)

(deleted), image/png
Details
(deleted), image/png
shorlander
: ui-review+
Details
(deleted), patch
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100610 Minefield/3.7a5pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100610 Minefield/3.7a5pre I'm using the current nightly build and noticed that the back button behavior is different than in all previous versions of Firefox (from 1.0 until 3.6.3) With a maximized window, the back button should be click-able from the extreme left edge of the screen, next to the button. This is to take advantage of Fitts's law. This is how it works in all previous Firefox versions (and most other browsers), and I consider it a bug in the current nightly version. This report is just to ensure awareness, I realize there are hundreds of more pressing issues at the moment. Reproducible: Always Steps to Reproduce: 1. Maximize window 2. Make sure the back button is active (ie, there's a previous page to go to) 3. Click with mouse directly against the left edge of the screen, at the same vertical position as the back button Actual Results: Nothing happens. Expected Results: The back button should be activated the same as if I clicked directly on it.
I can reproduce this on 3.7a6pre for both small and large icons. It has probably been around since the new navigation icons landed.
I should have noted that this was on WinXP. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100614 Minefield/3.7a6pre
Here is a mockup for a possible solution to this issue. The back button is moved slightly left so that the clickable area is on the edge of the screen. It does look somewhat awkward though. The only other solution that comes to mind would be to increase the size of the clickable areas for the toolbar buttons to pre-3.7/4.0 levels. I'm not sure which one would be easier to implement. Sorry for the triple post.
For similar reasons the Bookmarks button (or whatever button happens to be the right-most) should probably be aligned to the right screen edge.
We don't have to move the actual button, we should just make the clickable area larger. Stephen, can you make sure this is connected to the theme bugs?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Interesting I noticed this to. I agree with limi, I would only increase the click-able area as moving or increasing the size of the actual icon will make it look and feel crammed.
This should also apply to the bookmarks toolbar.
As of Firefox 4 beta 9, this bug still exists. Any progress on fixing it?
(In reply to comment #8) > As of Firefox 4 beta 9, this bug still exists. Any progress on fixing it? This is the only bug I've seen posted for this issue. So unless someone is assigned and actively working on getting this fixed, probably not. Its not a blocker yet.. but it could fall under papercuts or polish.
Summary: Back button in 3.7a5 minefield does not work along edge of screen → Back button does not work along edge of screen (would like to see Fitts Law applied when on left most edge of the toolbars)
Version: unspecified → Trunk
I would like us to try to take a fix for this before RC, even just off of the circle it doesn't activate as a hit right now.
Still present in release, and this is a constant annoyance.
Component: Toolbars → Theme
QA Contact: toolbars → theme
Assignee: nobody → jwein
Status: NEW → ASSIGNED
Attached patch Patch for bug 571454 (obsolete) (deleted) — Splinter Review
Can you please take a look at my proposed change? I have not made the required changes to pinstripe and gnomestripe yet, but I have it very close to working on winstripe. There is an issue with the overlapping area of the back and forward buttons. It appears that the forward button has a higher z-index, and I have not come up with a way to fix this. Please let me know if you have any ideas. Thanks!
Attachment #541411 - Flags: feedback?(dao)
Attached patch Current progress of patch for 571454 (obsolete) (deleted) — Splinter Review
This is an update of the current progress for the fix of this bug. The clickable area is painted red and the z-index issue has been fixed. This still needs to be ported to gnomestripe and pinstripe, as well as moving theme-specific styles out of the generic browser.css file.
Attachment #541411 - Attachment is obsolete: true
Attachment #541592 - Flags: feedback?(dao)
Attachment #541411 - Flags: feedback?(dao)
Nothing needs to be done for gnomestripe, as far as I can tell. Pinstripe should be a separate bug.
Attached patch Patch for bug 571454 (obsolete) (deleted) — Splinter Review
This patch increases the clickable area for the back button on Windows and Mac. Gnomestripe had to be updated because browser.xul changed.
Attachment #541592 - Attachment is obsolete: true
Attachment #542971 - Flags: review?(dao)
Attachment #541592 - Flags: feedback?(dao)
Dão, do you think we'll be able to land this patch in time for Firefox 7?
Probably not. I'm not happy with this patch, the XUL markup in particular doesn't seem sound.
Attachment #542971 - Flags: feedback?(fryn)
Attached patch Patch for bug 571454 - version 2 (obsolete) (deleted) — Splinter Review
I have redone the patch without modifying browser.xul by repurposing the nested toolbarbutton-icon. This patch only applies the changes to winstripe. I think this is a much cleaner solution for the bug than the previous patch that I submitted. Thanks Dão for pushing me :)
Attachment #542971 - Attachment is obsolete: true
Attachment #543546 - Flags: review?(dao)
Attachment #543546 - Flags: feedback?(fryn)
Attachment #542971 - Flags: review?(dao)
Attachment #542971 - Flags: feedback?(fryn)
Comment on attachment 543546 [details] [diff] [review] Patch for bug 571454 - version 2 >+#navigator-toolbox[iconsize="small"][mode="icons"] > #nav-bar #back-button.toolbarbutton-1 { >+ -moz-margin-start: 2px; >+ -moz-margin-end: 0; >+} Why this change? >-#back-button { >+#back-button > .toolbarbutton-icon { > -moz-image-region: rect(0, 18px, 18px, 0); > } > > #forward-button { > -moz-image-region: rect(0, 36px, 18px, 18px); > } > >-#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button { >+#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button > .toolbarbutton-icon { > -moz-image-region: rect(18px, 20px, 38px, 0); > } > > #back-button:-moz-locale-dir(rtl) > .toolbarbutton-icon, > #forward-button:-moz-locale-dir(rtl), > #forward-button:-moz-locale-dir(rtl) > .toolbarbutton-text { > -moz-transform: scaleX(-1); > } > >-#nav-bar #back-button { >+#nav-bar #back-button > .toolbarbutton-icon { > -moz-margin-end: 0 !important; > } And why these changes? >+#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button > .toolbarbutton-icon { > border-radius: 10000px; >- padding: 0; >+ padding: 5px; > width: 30px; > height: 30px; width/height aren't needed anymore when you set the padding, are they?
I have fixed the issues that you pointed out in your previous review. Many of the child selectors for |.toolbarbutton-icon| were unnecessary. However, there is an issue with this patch that I am not sure how to fix. Maybe you will have an idea: With large icons and the back button disabled, the list-item-image opacity is not properly changed to |opacity: .5|. This is because |.toolbarbutton-icon| has been re-purposed for the visual state of the button itself, and thus needs |opacity: .8|. Do you know if/how I could set the list-item-image to |opacity: .5|?
Attachment #543546 - Attachment is obsolete: true
Attachment #543697 - Flags: feedback?(dao)
Attachment #543546 - Flags: review?(dao)
Attachment #543546 - Flags: feedback?(fryn)
One way I have thought would be possible would be to just change the opacity of the |.toolbarbutton-icon| to .5 and then adjust the background-colors, box-shadows, and border-colors to fit, but that seems like it will be pretty hard to get correct.
We could stop reducing the icon's opacity on top of reducing the button's opacity. I.e. just use opacity:0.4 for the button and leave the icon alone.
Attachment #543697 - Flags: feedback?(dao)
Comment on attachment 543697 [details] [diff] [review] Patch for bug 571454 - version 3 (with minor regression) > #navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button { >+ margin: -5px 0; >+#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button > .toolbarbutton-icon { > margin-top: -2px; > margin-bottom: -2px; Why are you setting negative margins on the icon?
(In reply to comment #23) > Comment on attachment 543697 [details] [diff] [review] [review] > Patch for bug 571454 - version 3 (with minor regression) > > > #navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button { > >+ margin: -5px 0; > > >+#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button > .toolbarbutton-icon { > > margin-top: -2px; > > margin-bottom: -2px; > > Why are you setting negative margins on the icon? Previously, the #back-button had the -2px margins. The styling was moved to the icon, so that's where the -2px comes from. The #back-button itself has the larger negative margins so that the clickable area will extend slightly above and below the icon.
I still don't see why the -2px margins need to move over to the icon when the button gets larger negative margins.
(In reply to comment #25) > I still don't see why the -2px margins need to move over to the icon when > the button gets larger negative margins. Good catch. I just removed the negative margins on the icon and there is no change in appearance. I should have tried it before.
Attached patch Possible patch for bug 571454 - version 3.1 (obsolete) (deleted) — Splinter Review
I have removed the negative margins on the icon. I tried changing the opacity of the button to .4 and removing the opacity changes of the icon. This change gave a washed out look to the back button that is different than the disabled look of the other toolbar buttons. One approach for changing the opacity of the list-style-image, is simply to include a "disabled" state of the large back arrow in Toolbar.png. I have made a crude attempt at demonstrating it in this patch. While we don't include any explicit disabled states of images in the Toolbar.png, it would be a simple solution for this problem. What do you think?
Attachment #544326 - Flags: feedback?(dao)
> I tried changing the opacity of the button to .4 and removing the opacity > changes of the icon. This change gave a washed out look to the back button > that is different than the disabled look of the other toolbar buttons. I meant doing this for all buttons, not just the back button.
Attached patch Patch for bug 571454 - version 3.2 (obsolete) (deleted) — Splinter Review
Attachment #544333 - Flags: review?(dao)
Comment on attachment 544333 [details] [diff] [review] Patch for bug 571454 - version 3.2 I have made all toolbar buttons have an opacity of .4 when disabled.
Attachment #544326 - Attachment is obsolete: true
Attachment #544326 - Flags: feedback?(dao)
Attachment #543697 - Attachment is obsolete: true
Attached image Screenshot of change to disabled button opacity (obsolete) (deleted) —
I would like to double check to see if there are any qualms about the change to the opacity on the disabled toolbar buttons. I have included a screenshot of disabled buttons along with the Paste button enabled.
Attachment #544360 - Flags: ui-review?(shorlander)
Comment on attachment 544333 [details] [diff] [review] Patch for bug 571454 - version 3.2 > #nav-bar .toolbarbutton-1[disabled="true"] { >- opacity: .8; >+ opacity: .4; > } .5 might be sufficient as well. > #nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-button:not([disabled="true"]):not(:active):hover, > #nav-bar .toolbarbutton-1:not([open="true"]):not(:active):hover > .toolbarbutton-menubutton-dropmarker:not([disabled="true"]), >-#nav-bar .toolbarbutton-1:not([type="menu-button"]):not([disabled="true"]):not([checked="true"]):not([open="true"]):not(:active):hover { >+#nav-bar .toolbarbutton-1:not([type="menu-button"]):not([disabled="true"]):not([checked="true"]):not([open="true"]):not(:active):hover, >+#navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button:not([disabled="true"]):not(:active):hover > .toolbarbutton-icon { This new selector also needs :not([open]). >-#nav-bar #back-button { >+#navigator-toolbox[iconsize="small"][mode="icons"] > #nav-bar #back-button { > -moz-margin-end: 0 !important; > } This change looks unnecessary. You're affecting mode=text and mode=full here. > #navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button { >- border-radius: 10000px; >- padding: 0; >- width: 30px; >- height: 30px; >+ margin: -5px 0; >+ padding-top: 0; >+ padding-bottom: 0; >+ -moz-padding-start: 5px; >+ -moz-padding-end: 0; > position: relative; > z-index: 1; >- margin-top: -2px; >- margin-bottom: -2px; >+ border-radius: 0 10000px 10000px 0; >+ background: transparent; >+ border: none; >+ box-shadow: none; >+ text-shadow: none; >+} text-shadow:none isn't needed, since there's no text.
Attachment #544333 - Flags: review?(dao) → review+
Attached patch Patch for bug 571454 (obsolete) (deleted) — Splinter Review
I have made the requested changes. Pending shorlander's approval, can you land this for me Dão?
Attachment #544333 - Attachment is obsolete: true
I switched to an opacity of .5 for the disabled buttons, and have included a screenshot of the buttons at .4 opacity for comparison. Stephen: Can you please let me know if you are OK with this change?
Attachment #544360 - Attachment is obsolete: true
Attachment #544883 - Flags: ui-review?(shorlander)
Attachment #544360 - Flags: ui-review?(shorlander)
Comment on attachment 544883 [details] Screenshot of different opacities for disabled buttons .4 Opacity works well. It is actually an improvement because it makes it more obvious :)
Attachment #544883 - Flags: ui-review?(shorlander) → ui-review+
Attached patch Patch for bug 571454 (deleted) — Splinter Review
Attachment #544882 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
No longer blocks: 544821
This patch appears to fail when small toolbar icons are used. Is this intentional?
(In reply to comment #38) > This patch appears to fail when small toolbar icons are used. Is this > intentional? This was not intentional per se, but it was known. I have filed bug 670565 to track this issue with small icons.
I'm not sure if this is the appropriate place to ask this question, but I'm using a userchrome.css file that contains the following code: #nav-bar { padding: 6px !important; } This patch fails unless the above code segment is commented out. What should I do to preserve the effects of the above code AND this patch at the same time?
(In reply to comment #40) > I'm not sure if this is the appropriate place to ask this question, but I'm > using a userchrome.css file that contains the following code: I don't think this is the appropriate place for this discussion. Unfortunately, I'm not sure where the appropriate place would be either. > #nav-bar { > padding: 6px !important; > } > > This patch fails unless the above code segment is commented out. > > What should I do to preserve the effects of the above code AND this patch at > the same time? I'm not sure what else is in the userchrome.css file, but you could try adding this to the line below your padding rule: #nav-bar { padding: 6px !important; -moz-padding-start: 0 !important; } You would then have to add the following rule to your userchrome.css file: #navigator-toolbox[iconsize="large"][mode="icons"] > #nav-bar #back-button { -moz-padding-start: 11px; }
Thanks; appreciate the help.
Surely a better idea would have been to include some spacers on the far ends of the nav bar that copy the onClick events from the toolbar buttons before or after them (depending on whether they are a start or end spacer). This way it is all automatic. I feel the current method is somewhat "hackish" because it is skinning the .toolbarbutton-icon, which means all themes will be incompatible, and add-ons like Stratiform have a very hard time keeping up with a change like this, which doesn't even work for any buttons other than the back button. I'd give some code or a patch but I'm horrible with JavaScript.
Depends on: 671111
Blocks: 683494
Sorry if this is a stupid question, but is the fix here simply moving the button away from the edge of the screen? If so, I think I can call this verified on Firefox 8.0b1.
Whiteboard: [qa+][testday-20110930]
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #44) > Sorry if this is a stupid question, but is the fix here simply moving the > button away from the edge of the screen? If so, I think I can call this > verified on Firefox 8.0b1. No. Here's how you verify it: 1. Maximize the window. 2. Make the back button enabled if it isn't already (by following a link, etc.). 3. Click part of the left edge of the screen that is to the left of the back button. If the back button activates, this bug is fixed.
So I can confirm this is the behaviour on Windows 7 but not any other platforms. Should this not be a unified experience across all platforms?
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #46) > So I can confirm this is the behaviour on Windows 7 but not any other > platforms. Should this not be a unified experience across all platforms? This bug was only meant to be fixed for Windows XP and higher. Other platforms don't have these types of affordances (such as Mac OSX where dragging part of the toolbar will drag the window).
OS: Windows 7 → Windows XP
(In reply to Jared Wein [:jwein] from comment #47) > Other platforms don't have these types of affordances (such as Mac OSX where > dragging part of the toolbar will drag the window). How unfortunate...
Status: RESOLVED → VERIFIED
Keywords: verified-beta
Whiteboard: [qa+][testday-20110930] → [qa!][testday-20110930]
Great that this has been fixed in Firefox 8 beta! Any chance though that this can be extended to work for *any* toolbar item that sits adjacent to *either* the left or right screen edges? (the Google Chrome "wrench" menu, for example, can be activated from the right screen edge)
(In reply to broccauley from comment #49) > Any chance though that this can be extended to work for *any* toolbar item > that sits adjacent to *either* the left or right screen edges? Yes, that makes total sense. Can you file a new bug for that?
Bug 941046 added a test for this.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: