Closed Bug 544816 Opened 13 years ago Closed 12 years ago

Attach combined Stop/Go/Refresh button to the Location Bar

Categories

(Firefox :: Toolbars and Customization, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b7
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: shorlander, Assigned: fryn)

References

(Depends on 1 open bug)

Details

(Keywords: icon, user-doc-needed)

Attachments

(8 files, 10 obsolete files)

41.67 KB, image/png
Details
37.07 KB, image/png
Details
27.36 KB, patch
Details | Diff | Splinter Review
3.15 KB, patch
Details | Diff | Splinter Review
379.94 KB, image/png
Details
25.44 KB, image/jpeg
Details
28.66 KB, image/png
Details
46.68 KB, image/png
Details
For the new Firefox theme/UI we need a combined Stop/Go/Refresh button that is attached to the end of the Location Bar.
			
* If a page can be refreshed it appears as a glyph on the location bar background
* If it can't be refreshed the glyph is greyed out
* The refresh state turns blue on hover
* If you enter the location bar and start typing it will turn green with a go arrow
* Turns into a red "x" stop button when the page load can be stopped
* Slight gap between when a page is finished loading and when you can refresh again (like the behavior currently in the 3.7 nighties)

It should be removable via the Customization dialog. Ideally it would separate itself visually from the location bar in this mode.

See attachment for mockup.
Blocks: 544820
Blocks: 544821
Target Milestone: Future → ---
I like the idea: it just seems to fit everything here.
When would the button appear greyed out? Is that just in the "slight gap" when the page finishes loading? Can you still force a refresh during that "slight gap" by using the F5 button?

I personally prefer to have stop/reload over with the other navigation buttons, but as long as I have the ability to go back to that option, I can't really complain about it being changed.
I find the idea of having that single button there for Stop/Go/Refresh very good, it seems it could work properly. Specially, I like transforming the button to a "go" arrow when the user is typing. 

But... don't you think that indicating the function change on the button with four different colors could be too distracting for the user? I mean, it could be drawing too much attention in moments that are maybe not so needed. 

Maybe users would anyways see it if you use one color, and the interface would look cleaner/less distracting, allowing the user to concentrate on the content.  

[I have found this bug after making a comment on the same topic at the Stackexchange (http://mozilla.stackexchange.com/questions/51/merge-stop-and-reload-buttons-as-one/83#83).]
(In reply to comment #3)
> But... don't you think that indicating the function change on the button with
> four different colors could be too distracting for the user? I mean, it could
> be drawing too much attention in moments that are maybe not so needed. 

I don't think so. Research have shown that colors are easier to quickly take in, rather than symbols, for the brain. Also, the changes would not occur that much, and I have a hard time imagining that would be distracting. When we'll get this into trunk we can get feedback on if it distracts at all.
(In reply to comment #3)
But... don't you think that indicating the function change on the button with
> four different colors could be too distracting for the user? I mean, it could
> be drawing too much attention in moments that are maybe not so needed. 

I'm using the "Strata40" family of extensions and it hasn't detracted me at all. I don't think it would be a problem.
I think it would be a nice a idea to test this before releasing it. Specially with Mac users; do we want Firefox to match the look of the OS? The colors thing looks more windowsish... OK, but maybe that's what the mockup is actually trying to show.

My issue with the colors is just a detail, still I would be careful with it.
I'm little worried that the color change might be too distracting when it is occurring off in peripheral vision and you are otherwise focused on the page.  We'll get a good sense as we test the theme in nightly builds.

>The colors thing looks more windowsish

The visual style there is specific to windows, here is an OS X mockup showing monochrome controls: https://wiki.mozilla.org/images/0/02/Firefox-4-Mockup-i05-%28OSX%29-%28TabsTop%29-%28Default%29-%28Small%29.png
Thanks Alex for your comment.

I like that OS X mockup... hopefully the tabs stay in that position (I guess that's another topic).
Blocks: 572482
Seems like everybody forgot about this.
(In reply to comment #10)
This isn't targeted for beta 1, so no one needs to work on it yet.

https://wiki.mozilla.org/Firefox/Projects/New_Theme/Timeline

There are still more important bugs that haven't landed yet, such as bug 544817. The developers will want to finish them before someone ends up doing this one.
Blocks: 578025
Latest Chrome development builds moved Stop/Reload to where we have it. It would be interesting to know their motivation...
"Stop and Reload have been combined, and Go eliminated, to make things simpler and keep all the navigation-related toolbar buttons together."

http://blog.chromium.org/2010/06/fresh-coat-of-chrome.html
If Dão or someone else has already started on this, please let me know and take it over; otherwise, I'll look into this.

The back and forward buttons are designed to navigate between pages in history, whereas the location bar and the stop/reload button relate to the current page.
The go button is just there as a concession to old, mouse-driven UI, where forms all have a submit button.
>Latest Chrome development builds moved Stop/Reload to where we have it. It
>would be interesting to know their motivation...

I would like to know this as well.  It would be pretty ironic if their primary motivation was to create an interface that was more familiar for transitioning Firefox users.  Either way, we are interested in applying the same general logic, and easily transitioning over IE7 and IE8 users.

Also with an integrated progress bar the controls have a good natural mapping to the animation.
Assignee: nobody → fyan
Status: NEW → ASSIGNED
this works 100% on Mac, if you want to try out this patch.
steps to try:
1. build with patch and start minefield
2. remove the standalone reload and stop buttons from your toolbar
3. voila! the awesome combined widget appears!

remaining blocking issues:
1. style it up for windows and linux
2. write code so that upon version upgrade, it sets the combined widget as the default if the user's toolbar mode is icons only
3. use shorlander's shiny icons
see instructions as v0.1 to try at your own risk.
windows styling may still be broken.
Attachment #457225 - Attachment is obsolete: true
(In reply to comment #16)
> Created attachment 457225 [details] [diff] [review]
> WIP v0.1 (Mac only; uses existing icons)
> 2. remove the standalone reload and stop buttons from your toolbar

I think moving the stop/reload buttons BEHIND the locationbar should active the combined buttons. Removing them should just remove them. No p
> I think moving the stop/reload buttons BEHIND the locationbar should active the
> combined buttons. Removing them should just remove them. No p

There are technical difficulties blocking this, but I think I have a workaround.
v0.3 will solve this.
NEW steps to try:
1. build with patch and start minefield
2. move the toolbar icons so that the location bar, the reload button, and the stop button appear adjacent in exactly that order
3. voila! the awesome combined widget appears!

remaining blocking issues:
1. finish styling it up on windows and linux
2. use shorlander's shiny icons and gradients
3. upon version upgrade, it moves the toolbar icons so that the combined widget is the default (do this movement only if the toolbar mode is icons)
Attachment #457272 - Attachment is obsolete: true
Attachment #457382 - Attachment is patch: true
Attachment #457382 - Attachment mime type: application/octet-stream → text/plain
I haven't tested it yet on Windows and Linux, but based on the code, it looks like it should work on those platforms too. Same instructions as patch v0.3 to try out.
Attachment #457382 - Attachment is obsolete: true
only change since v0.4: fixed the location bar sizing regression.
Attachment #457540 - Attachment is obsolete: true
Attached image WIP 5 on Linux (obsolete) —
I just built latest central code with the latest wip patch and the result is interesting :)

Basicly, the icons on the right side all belong to one button (reload/stop) so the patch basicly works, only the styling is messed up a bit
(In reply to comment #23)
> I just built latest central code with the latest wip patch and the result is
> interesting :)
> 
> Basicly, the icons on the right side all belong to one button (reload/stop) so
> the patch basicly works, only the styling is messed up a bit

Thanks for the feedback. I was just about to try it myself. That will be fixed in the next iteration :)
(In reply to comment #16)
> remaining blocking issues:
> 1. style it up for windows and linux
> 2. write code so that upon version upgrade, it sets the combined widget as the
> default if the user's toolbar mode is icons only
> 3. use shorlander's shiny icons
Plus 4. Tests for various behaviors, right?
Blocks: 578967
I'm not sure if this is the right place...

I like the idea, but I am curious why the decision was made to put it on the right side of the address bar.  Less and less emphasis is being put on top right hand area of the browser, especially with the addition of the new Firefox button.

In my opinion, to match the growing trend of keeping all of the icons/tabs/buttons in the top left are of the browser, putting the combined go/stop/refresh button makes considerable more sense to be put on the left side of the address bar.  Not to mention it will be less drastic of a change for most users, so the change won't be as large of a shock.

Also, with the proliferation of wide screen monitors and higher resolutions, the most important functionality (the left side buttons) is drawn even further away, making the go/stop/refresh button even more of a chore to click than it was before.  The larger the resolution of the screen becomes, the longer it takes for the mouse to travel to the rightside.  And this combined with the smaller button size makes clicking it more frustrating.

At the very least I think the combined button should be configurable to reside on both sides of the address bar, however I think it would be more suitable to make a left-sided button the default
I think Chris has a few good points here. I want to go further into the "less drastic of a change" point: as far as I can see the bookmark star is going to be on the left, so it changes place with those other commands, basicly (not counting "go"). Chrome recently changed from "Star on the left" to "Star on the right", I don't know if there are any special reasons for this but they may have studies which show that the current way (star on the right) works better? (or they just tried to copy Fx 3.x, which would be a surprise to me)
I agree with the last two comments. Firefox by default has the stop/reload button right after the back/forward button. Having the stop/reload on the right, it takes too long to move the cursor to the far right, especially on widescreen monitors. And with the FF button to the top left, it'd make more sense to have the reload button also. As for the Go button, I don't see the point in having it. It should be kept as a small arrow at the end of the url.
The question is : where is your mouse pointer when you want to reload the page ?
(In reply to comment #29)
> The question is : where is your mouse pointer when you want to reload the page
> ?

? Usually in the middle of the screen. Certainly not behind the locationbar.
so the argument that it takes too long to move the cursor is irrelevant
(In reply to comment #31)
> so the argument that it takes too long to move the cursor is irrelevant

Nice counterargument. That was sarcasm by the way.

Disregarding that, why should the page controls be split up? Why not just have them in the same left side of the navigation bar? And as you can see in numerous mockups, the new bookmarks button's supposed to be behind the location. 

So now we have the back/forward button right next to the bookmark star, and the reload/stop/go button right next to the bookmark menu.


mmm, inconsistent much?
(In reply to comment #32)
> Disregarding that, why should the page controls be split up? Why not just have
> them in the same left side of the navigation bar?

I think the argument (and I agree with it) is that "Go" is not a 'page control' in the same way and must be attached to the URL bar.  This means you can do it on the right or the left but the left is already taken up by page security information.  It also makes sense to have go at the end of the URL.

If that logic is kept then your options are to put the Stop/Go/Refresh on the right (as per mockup) or to separate out Go from the other two.
(In reply to comment #33)
> (In reply to comment #32)
> > Disregarding that, why should the page controls be split up? Why not just have
> > them in the same left side of the navigation bar?
> 
> I think the argument (and I agree with it) is that "Go" is not a 'page control'
> in the same way and must be attached to the URL bar.  This means you can do it
> on the right or the left but the left is already taken up by page security
> information.  It also makes sense to have go at the end of the URL.
> 
> If that logic is kept then your options are to put the Stop/Go/Refresh on the
> right (as per mockup) or to separate out Go from the other two.

That's basically what I mean. The stop/reload buttons should be kept together attached to the left of the urlbar, and the go button should stay inside the url bar as the small blue arrow following the url.
Thanks for the input guys; I can see how this issue is debatable, and I think the most friendly (albeit longest) solution would be to make it draggable/toggleable for both sides.  Ultimately it's up to the implementers, but hopefully this is taken into consideration :]

Btw, who actually uses the Go button anyways? :P
The Go button is mouse driven, rather than having to hit enter.  I added an extension just so I can use it again.
(In reply to comment #34)

The userstyle here illustrates what I mean.
http://userstyles.org/styles/34002
Blocks: 579521
Attached patch patch w/ glyphs by shorlander (obsolete) — Splinter Review
Attachment #457545 - Attachment is obsolete: true
Attachment #457605 - Attachment is obsolete: true
Attachment #463381 - Flags: ui-review?(shorlander)
Attachment #463381 - Flags: review?(dolske)
Comment on attachment 463381 [details] [diff] [review]
patch w/ glyphs by shorlander

- breaks resizing the location and search bar
- lacks the Go arrow when stop and reload are elsewhere

this is also not okay:

/* i still need to check if this actually works in RTL locales */
(In reply to comment #39)
> /* i still need to check if this actually works in RTL locales */

Let me know if you need help there.
Attachment #463381 - Attachment is obsolete: true
Attachment #464589 - Flags: ui-review?(shorlander)
Attachment #464589 - Flags: review?(dao)
Attachment #463381 - Flags: ui-review?(shorlander)
Attachment #463381 - Flags: review?(dolske)
Comment on attachment 464592 [details]
screenshot of patch v7 w/ Force RTL

Looks great!
Attached patch patch v8 (typo fix) (obsolete) — Splinter Review
Attachment #464589 - Attachment is obsolete: true
Attachment #464616 - Flags: ui-review?(shorlander)
Attachment #464616 - Flags: review?(dao)
Attachment #464589 - Flags: ui-review?(shorlander)
Attachment #464589 - Flags: review?(dao)
Attachment #464616 - Attachment is obsolete: true
Attachment #464626 - Flags: ui-review?(shorlander)
Attachment #464626 - Flags: review?(dao)
Attachment #464616 - Flags: ui-review?(shorlander)
Attachment #464616 - Flags: review?(dao)
v9 does not compile on linux:

File "reload-stop-go.png" not found in themes/gnomestripe/browser (from JarMaker)
(In reply to comment #46)
> v9 does not compile on linux:
> 
> File "reload-stop-go.png" not found in themes/gnomestripe/browser (from
> JarMaker)

This patch applies just fine for me on all platforms, so I don't know what you're running into. I checked the diff too, to make sure that I'm adding the png file to the repo.
Attachment #464626 - Flags: review?(dolske)
I've tested this on tryserver, and it passed all non-intermittently failing tests.
Attachment #464626 - Flags: ui-review?(shorlander) → ui-review+
Attachment #464626 - Flags: review?(dolske)
Attachment #464626 - Flags: review?(dao)
Attachment #464626 - Flags: review+
Attachment #464626 - Flags: approval2.0+
Keywords: checkin-needed
Attachment #464626 - Attachment is obsolete: true
Concrete numbers for db6e503fec9f vs. 1504917ed42e:

Linux     95.37   99.37  (+4%)
Linux64   93.28   99.32  (+6%)
OS X     178.53  184.95  (+3%)
OS X64   252.26  253.95  (+1%)
WinXP     80.89   82.63  (+2%)
Win7     106.16  107.18  (+1%)

Comparisons of 1504917ed42e with revisions preceding db6e503fec9f looked similar. Normal twinopen fluctuation on Linux appeared to be +/- 2%.
Hovering the combined button doesn't give any animation, like fading-in and fading out.
(In reply to comment #51)
> Concrete numbers for db6e503fec9f vs. 1504917ed42e:
Slightly larger range looks like this:
http://tinyurl.com/22pph2t

(using db6e503fec9f, 45716b17fb82, e151e2b04a0c against 1504917ed42e)
blocking2.0: --- → ?
So, the only code from the patch which looks possible to affect Twinopen is the CombinedStopReload.init() addition in browser.js.

2 possible improvements I see here, both of which I pushed to Try to measure.

1). Pre-add |combined="true"| to browser.xul, so that for the common case of having default UI this code is basically a no-op. I'd suspect this to be the most likely cause of the regression.

2) Avoid extra calls to getElementById by checking |foo.nextSibling.id == "bar"| instead of |foo.nextSibling == getElementById("bar")|. This doesn't seem as likely to be a regression cause [getElementById should already be fast], but was easy enough to try.

Relevant changesets:

A: bc95b9869d3f (mozilla-central parent for below)
X: 405199e84151 (try, original patch)
Y: 8163fb03bcff (try, patch + #1)
Z: fcbcf2520e97 (try, patch + #1 + #2)

A -> X
  Windows -1.25%
  Mac     -0.22%
  Mac 64   0.86%
  Linux    1.93%
  Linux 64 1.68%

A -> Y
  Windows    fail (gee, thanks)
  Mac        0.25%
  Mac 64     0.26%
  Linux     -0.25%
  Linux 64   0.20%

A -> Z doesn't really make sense:
  Windows -3.57%
  Mac     -0.60%
  Mac      0.04%
  Linux    0.80%
  Linux 64 0.39%

So, with the usual Talos handwaving, I think A -> Y looks to be within the normal noise, was most likely to be the issue anyway. A -> Z was inconclusive, less likely to have been an issue, and so probably isn't a change that needs to be made.

Will attach a combined patch, plan will be to reland original patch + |combined="true"|.
I want to ask if it is possible to de-attach the button from the location bar. sorry if I am disturbing your work.
blocking2.0: ? → beta6+
> I want to ask if it is possible to de-attach the button from the location bar.
> sorry if I am disturbing your work.


In customization mode, you'll be able to move the stop/go button like any regular one. I don't know about the go button though.

@Justin, are there any remaining performance issues or can you check this in in your spare time?
Pushed http://hg.mozilla.org/mozilla-central/rev/7aaf30721c48
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6
(In reply to comment #51)
> Concrete numbers for db6e503fec9f vs. 1504917ed42e:
> 
> Linux     95.37   99.37  (+4%)
> Linux64   93.28   99.32  (+6%)

TBH, I'm not so sure there was a regression in the original landing. Looking at the graph for these two (the two largest, attached as PNG), the data points look well within the usual noise.

Will still keep an eye on the perf numbers to see if anything pops up this time, though.
Attached image screenshot
Build : http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1283485308/

1px height difference.
load Acid3 and bugzilla.
Depends on: 593293
Attached image Button width is 2px off
Width is too small compared to mockups.

It seems to be about 2 pixels off and makes the button feel shorter.
(In reply to comment #60)

(In reply to comment #61)

Please put pixel pushing/polish-type bugs/screenshots here:
https://bugzilla.mozilla.org/show_bug.cgi?id=593293
(In reply to comment #39)
> Comment on attachment 463381 [details] [diff] [review]
> patch w/ glyphs by shorlander
> 
> - breaks resizing the location and search bar

Has this ever been addressed? Word on mozillazine has that it hasn't been.
Depends on: 593357
Depends on: 593350
(In reply to comment #63)
> (In reply to comment #39)
> > Comment on attachment 463381 [details] [diff] [review] [details]
> > patch w/ glyphs by shorlander
> > 
> > - breaks resizing the location and search bar
> 
> Has this ever been addressed? Word on mozillazine has that it hasn't been.

It works for me on Windows 7 and OS X.
Beautiful.

Ready to mark this one as VERIFIED FIXED and move on to followups?
Compare to mockup (https://wiki.mozilla.org/images/1/17/Firefox-4-Mockup-i06-%28Win7%29-%28Aero%29-%28TabsTop%29.png) it's too narrow. It needs at least one pixel in the width.
(In reply to comment #64)
> (In reply to comment #63)
> > (In reply to comment #39)
> > > Comment on attachment 463381 [details] [diff] [review] [details] [details]
> > > patch w/ glyphs by shorlander
> > > 
> > > - breaks resizing the location and search bar
> > 
> > Has this ever been addressed? Word on mozillazine has that it hasn't been.
> 
> It works for me on Windows 7 and OS X.

It doesn't work for me on Windows 7. Filed bug 593357
Depends on: 593392
(In reply to comment #66)
> Compare to mockup [...] It needs at least one pixel in the width.

Twiddling and tweaking is in bug 593293.
Keywords: user-doc-needed
Depends on: 593262
Depends on: 593640
(In reply to comment #67)
> (In reply to comment #64)
> > (In reply to comment #63)
> > > (In reply to comment #39)
> > > > Comment on attachment 463381 [details] [diff] [review] [details] [details] [details]
> > > > patch w/ glyphs by shorlander
> > > > 
> > > > - breaks resizing the location and search bar
> > > 
> > > Has this ever been addressed? Word on mozillazine has that it hasn't been.
> > 
> > It works for me on Windows 7 and OS X.
> 
> It doesn't work for me on Windows 7. Filed bug 593357

It works for me with a big *IF*: after I've entered (and aborted) toolbar customization. But opening a new window it doesn't work in there until toolbar customization is invoked again...
Depends on: 593684
Depends on: 593719
No longer depends on: 593640
No longer depends on: 593684
No longer depends on: 593350
No longer depends on: 593392
No longer depends on: 593719
Depends on: 606691
blocking2.0: beta7+ → beta8+
Depends on: 652363
Depends on: 686712
You need to log in before you can comment on or make changes to this bug.