Closed
Bug 544816
Opened 15 years ago
Closed 14 years ago
Attach combined Stop/Go/Refresh button to the Location Bar
Categories
(Firefox :: Toolbars and Customization, enhancement)
Firefox
Toolbars and Customization
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)
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.
Updated•15 years ago
|
Target Milestone: Future → ---
Comment 1•15 years ago
|
||
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.
Comment 3•15 years ago
|
||
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.
Comment 5•15 years ago
|
||
(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.
Comment 6•15 years ago
|
||
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.
Comment 7•15 years ago
|
||
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
Comment 8•15 years ago
|
||
Thanks Alex for your comment.
I like that OS X mockup... hopefully the tabs stay in that position (I guess that's another topic).
Comment 9•15 years ago
|
||
Yes, it's bug #544815
Comment 10•14 years ago
|
||
Seems like everybody forgot about this.
Comment 11•14 years ago
|
||
(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.
Comment 12•14 years ago
|
||
Latest Chrome development builds moved Stop/Reload to where we have it. It would be interesting to know their motivation...
Comment 13•14 years ago
|
||
"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
Assignee | ||
Comment 14•14 years ago
|
||
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.
Comment 15•14 years ago
|
||
>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 | ||
Updated•14 years ago
|
Assignee: nobody → fyan
Status: NEW → ASSIGNED
Assignee | ||
Comment 16•14 years ago
|
||
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
Assignee | ||
Comment 17•14 years ago
|
||
see instructions as v0.1 to try at your own risk.
windows styling may still be broken.
Attachment #457225 -
Attachment is obsolete: true
Comment 18•14 years ago
|
||
(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
Assignee | ||
Comment 19•14 years ago
|
||
> 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.
Assignee | ||
Comment 20•14 years ago
|
||
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
Assignee | ||
Updated•14 years ago
|
Attachment #457382 -
Attachment is patch: true
Attachment #457382 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 21•14 years ago
|
||
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
Assignee | ||
Comment 22•14 years ago
|
||
only change since v0.4: fixed the location bar sizing regression.
Attachment #457540 -
Attachment is obsolete: true
Comment 23•14 years ago
|
||
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
Assignee | ||
Comment 24•14 years ago
|
||
(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 :)
Comment 25•14 years ago
|
||
(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?
Comment 26•14 years ago
|
||
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
Comment 27•14 years ago
|
||
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)
Comment 28•14 years ago
|
||
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.
Comment 29•14 years ago
|
||
The question is : where is your mouse pointer when you want to reload the page ?
Comment 30•14 years ago
|
||
(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.
Comment 31•14 years ago
|
||
so the argument that it takes too long to move the cursor is irrelevant
Comment 32•14 years ago
|
||
(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?
Comment 33•14 years ago
|
||
(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.
Comment 34•14 years ago
|
||
(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.
Comment 35•14 years ago
|
||
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
Comment 36•14 years ago
|
||
The Go button is mouse driven, rather than having to hit enter. I added an extension just so I can use it again.
Comment 37•14 years ago
|
||
(In reply to comment #34)
The userstyle here illustrates what I mean.
http://userstyles.org/styles/34002
Assignee | ||
Comment 38•14 years ago
|
||
Attachment #457545 -
Attachment is obsolete: true
Attachment #457605 -
Attachment is obsolete: true
Attachment #463381 -
Flags: ui-review?(shorlander)
Attachment #463381 -
Flags: review?(dolske)
Comment 39•14 years ago
|
||
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 */
Comment 40•14 years ago
|
||
(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.
Assignee | ||
Comment 41•14 years ago
|
||
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)
Assignee | ||
Comment 42•14 years ago
|
||
Comment 43•14 years ago
|
||
Comment on attachment 464592 [details]
screenshot of patch v7 w/ Force RTL
Looks great!
Assignee | ||
Comment 44•14 years ago
|
||
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)
Assignee | ||
Comment 45•14 years ago
|
||
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)
Comment 46•14 years ago
|
||
v9 does not compile on linux:
File "reload-stop-go.png" not found in themes/gnomestripe/browser (from JarMaker)
Assignee | ||
Comment 47•14 years ago
|
||
(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.
Assignee | ||
Updated•14 years ago
|
Attachment #464626 -
Flags: review?(dolske)
Assignee | ||
Comment 48•14 years ago
|
||
I've tested this on tryserver, and it passed all non-intermittently failing tests.
Reporter | ||
Updated•14 years ago
|
Attachment #464626 -
Flags: ui-review?(shorlander) → ui-review+
Updated•14 years ago
|
Attachment #464626 -
Flags: review?(dolske)
Attachment #464626 -
Flags: review?(dao)
Attachment #464626 -
Flags: review+
Updated•14 years ago
|
Attachment #464626 -
Flags: approval2.0+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment 49•14 years ago
|
||
Attachment #464626 -
Attachment is obsolete: true
Comment 50•14 years ago
|
||
Pushed:
http://hg.mozilla.org/mozilla-central/rev/367d4e57cbd2
But backed out due what dao assures me is a 4% twinopen talos regression on
Linux:
http://hg.mozilla.org/mozilla-central/rev/4fbefecf050b
http://hg.mozilla.org/mozilla-central/rev/5fd4fd971b0e
Keywords: checkin-needed
Comment 51•14 years ago
|
||
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%.
Comment 52•14 years ago
|
||
Hovering the combined button doesn't give any animation, like fading-in and fading out.
Comment 53•14 years ago
|
||
(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)
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 54•14 years ago
|
||
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"|.
Comment 55•14 years ago
|
||
Comment 56•14 years ago
|
||
I want to ask if it is possible to de-attach the button from the location bar. sorry if I am disturbing your work.
Updated•14 years ago
|
blocking2.0: ? → beta6+
Comment 57•14 years ago
|
||
> 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?
Comment 58•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6
Comment 59•14 years ago
|
||
(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.
Comment 60•14 years ago
|
||
Build : http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1283485308/
1px height difference.
load Acid3 and bugzilla.
Comment 61•14 years ago
|
||
Width is too small compared to mockups.
It seems to be about 2 pixels off and makes the button feel shorter.
Assignee | ||
Comment 62•14 years ago
|
||
(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
Comment 63•14 years ago
|
||
(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.
Reporter | ||
Comment 64•14 years ago
|
||
(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.
Comment 65•14 years ago
|
||
Beautiful.
Ready to mark this one as VERIFIED FIXED and move on to followups?
Comment 66•14 years ago
|
||
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.
Comment 67•14 years ago
|
||
(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
Comment 68•14 years ago
|
||
(In reply to comment #66)
> Compare to mockup [...] It needs at least one pixel in the width.
Twiddling and tweaking is in bug 593293.
Updated•14 years ago
|
Keywords: user-doc-needed
Comment 69•14 years ago
|
||
(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...
Updated•14 years ago
|
blocking2.0: beta7+ → beta8+
You need to log in
before you can comment on or make changes to this bug.
Description
•