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

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Kurt Dixon]

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.

Comment 1

[MZ (watervole02)]

I can reproduce this on 3.7a6pre for both small and large icons. It has probably been around since the new navigation icons landed.

Comment 2

[MZ (watervole02)]

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

Comment 3

[MZ (watervole02)]

Attachment: Mockup of back button moved to left edge of screen

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.

Comment 4

[Paul McAuley]

For similar reasons the Bookmarks button (or whatever button happens to be the right-most) should probably be aligned to the right screen edge.

Comment 5

[Alex Limi (:limi) — Firefox UX Team]

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?

Comment 6

[Bojhan Somers]

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.

Comment 7

[imradyurrad]

This should also apply to the bookmarks toolbar.

Comment 8

[Kurt Dixon]

As of Firefox 4 beta 9, this bug still exists. Any progress on fixing it?

Comment 9

[[not reading bugmail]]

(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.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

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.

Comment 11

[Greg Edwards]

Still present in release, and this is a constant annoyance.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454

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!

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Current progress of patch for 571454

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.

Comment 14

[Dão Gottwald [:dao]]

Nothing needs to be done for gnomestripe, as far as I can tell. Pinstripe should be a separate bug.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454

This patch increases the clickable area for the back button on Windows and Mac. Gnomestripe had to be updated because browser.xul changed.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Dão, do you think we'll be able to land this patch in time for Firefox 7?

Comment 17

[Dão Gottwald [:dao]]

Probably not. I'm not happy with this patch, the XUL markup in particular doesn't seem sound.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454 - version 2

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 :)

Comment 19

[Dão Gottwald [:dao]]

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?

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454 - version 3 (with minor regression)

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|?

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 22

[Dão Gottwald [:dao]]

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.

Comment 23

[Dão Gottwald [: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?

Comment 24

[Jared Wein [:jaws] (please needinfo? me)]

(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.

Comment 25

[Dão Gottwald [:dao]]

I still don't see why the -2px margins need to move over to the icon when the button gets larger negative margins.

Comment 26

[Jared Wein [:jaws] (please needinfo? me)]

(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.

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Possible patch for bug 571454 - version 3.1

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?

Comment 28

[Dão Gottwald [: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.

Comment 29

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454 - version 3.2

Comment 30

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 31

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of change to disabled button opacity

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.

Comment 32

[Dão Gottwald [:dao]]

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.

Comment 33

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454

I have made the requested changes. Pending shorlander's approval, can you land this for me Dão?

Comment 34

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of different opacities for disabled buttons

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?

Comment 35

[Stephen Horlander [: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 :)

Comment 36

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch for bug 571454

Comment 37

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/1dcabbe1c7fc

Comment 38

[solcroft]

This patch appears to fail when small toolbar icons are used. Is this intentional?

Comment 39

[Jared Wein [:jaws] (please needinfo? me)]

(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.

Comment 40

[solcroft]

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?

Comment 41

[Jared Wein [:jaws] (please needinfo? me)]

(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;
}

Comment 42

[solcroft]

Thanks; appreciate the help.

Comment 43

[SpewBoy]

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.

Comment 44

[u279076]

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.

Comment 45

[Frank Yan (:fryn)]

(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.

Comment 46

[u279076]

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?

Comment 47

[Jared Wein [:jaws] (please needinfo? me)]

(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).

Comment 48

[u279076]

(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...

Comment 49

[Paul McAuley]

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)

Comment 50

[Alex Limi (:limi) — Firefox UX Team]

(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?

Comment 51

[Jared Wein [:jaws] (please needinfo? me)]

Bug 941046 added a test for this.