343396 - Merge Reload and Stop buttons when they are adjacent

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, enhancement)

Description

[catdogbeloved]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-GB; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-GB; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

Firefox's toolbar has one button to stop loading a page (cross icon), and a button for reloading the page (spiral icon). The problem is, that the cross icon  is displayed even if the page is not loading, and the spiral icon is displayed even when the page is already loading/reloading.

I suggest to have one smart button that 
- when loading a page, it shows the cross icon and stops the loading when pressed;
- when the page is loaded, it shows the spiral icon and reloads the page when pressed.

This allows for a more logical interface. It would also save space, as the smart button replaces the two dumb buttons.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Ria Klaassen (not reading all bugmail)]

*** This bug has been marked as a duplicate of 243244 ***

Comment 2

[catdogbeloved]

What follows is a copy of my post at
https://bugzilla.mozilla.org/show_bug.cgi?id=243244

--- begin -------
I've posted the same request for feature and redirected here.

My mail was: https://bugzilla.mozilla.org/show_bug.cgi?id=343396

As I read this thread, I can see that some of us want the feature, while others do not. In order to please both, I propose to make the smart button as an addition, not a replacement of the existing ones, and leave to people the freedom to choose what the like best.

Bob

PS. I tried to reopen this thread, but I got the following: "You tried to change the Status field from VERIFIED to REOPENED , but only the assignee or reporter of the bug, or a sufficiently empowered user may change that field." As I am not the owner of this thread, I am now trying to reopen my own.

--- end -----

Bugzilla clearly fails to allow me to reopen the thread to which it has redirected me in the first place. I then reopen my own thread, and invite people to follow up.

Comment 3

[Simon Bünzli]

This enhancement request doesn't sound like something the majority of users could take advantage of ("what's wrong with the existing Stop/Reload buttons?") but rather something which might even confuse them ("why is there another Reload button in the Customize window?"). Since there are good reasons for not having this behavior by default as mentioned in bug 243244 and Firefox should not unnecessarily be bloated, I'd rather see this as an extension (which in fact already exists). Suggesting WONTFIX.

Comment 4

[catdogbeloved]

Builtin or extension is a detail. I just want it, and I do not care whether someone "believes" that the majority of users would not benefit from it.
I am a user, and I want it.

I am now trying the following:

https://addons.mozilla.org/firefox/313/

Comment 5

[Jurriaan Mous]

I myself are a great fan of one stop/reload button and am using the extension for it. But with Safari converts and the Mac principle of remove any unneeded button I think it is necessary.

And with a clear description in customize it could work. :)

Comment 6

[catdogbeloved]

If one wants a statistics on how many people want this feature, the number of downloads of this and similar plugins says it all.

https://addons.mozilla.org/en-US/firefox/addon/313
https://addons.mozilla.org/en-US/firefox/addon/1342

I am happy with the plugin, but the very question is whether it could be part of the default. My judgment is that the stop-reload button is smart. I also like the "GrApple (eos)" interface, which I do not recommend as general default, but I would strongly recommend it as default for the osx binary.

Comment 7

[Dão Gottwald [:dao]]

Attachment: proposed patch

This combines Stop and Reload when they are adjacent, so users are still allowed to have separate buttons.

Try server builds:
https://build.mozilla.org/tryserver-builds/2007-11-18_17:[e-mail]-1195436432/

Comment 8

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 9

[Dão Gottwald [:dao]]

new try server builds:
https://build.mozilla.org/tryserver-builds/2007-11-18_17:[e-mail]-1195436432/

Comment 10

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #6)
> If one wants a statistics on how many people want this feature, the number of
> downloads of this and similar plugins says it all.
> 
400 million Stop/Reload extension downloads would convince me. Then it is advantage for everyone, for they don't need to update the extension anymore; the Firefox code would take care for this.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Definitely not a blocker.

Connor and I have been playing with the builds, and aren't sure that the behaviour is quite right yet. Worth continuing to play with, but not yet sure we want to do this.

Comment 12

[Dão Gottwald [:dao]]

Attachment: patch v3

faster transition, based on e-mail feedback from beltzner

https://build.mozilla.org/tryserver-builds/2007-12-01_02:[e-mail]-1196503164/

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 290986 [details] [diff] [review]
patch v3

After trying this out for a while, I'm pretty sure that we still don't want to do this.

Comment 15

[Dão Gottwald [:dao]]

Attachment: v4 (extension)

this is without fading

Comment 16

[Nicolas Mandil (:NicolasWeb)]

(In reply to comment #14)
> (From update of attachment 290986 [details] [diff] [review])
> After trying this out for a while, I'm pretty sure that we still don't want to
> do this.
Could you please explain us what was displeasant in your try of  this functionnality ? Maybe we can solve them by this way (improve function) or an other...

Comment 17

[Dão Gottwald [:dao]]

I think the fading was a little bit distracting. I definitely like it better without it.

I also think this would simplify our UI significantly.

Comment 18

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

>I think the fading was a little bit distracting.

The fading is distracting, but for usability we do want to reduce the number of accidental reloads when trying to stop.  Perhaps cycle stop->disabled stop [short time]->reload?

>I also think this would simplify our UI significantly.

yeah, now that the home button is going back up to the navigation toolbar, it would be nice to get this landed to maintain simplicity. Are there any issues preventing v4 from potentially being used?

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #18)
> The fading is distracting, but for usability we do want to reduce the number of
> accidental reloads when trying to stop.  Perhaps cycle stop->disabled stop
> [short time]->reload?

v4 does that, with a "short time" of 500 ms.

> yeah, now that the home button is going back up to the navigation toolbar, it
> would be nice to get this landed to maintain simplicity. Are there any issues
> preventing v4 from potentially being used?

Technically, I think the way we would do this implies a Txul hit, which would hopefully be small enough to be outweighed by the simplicity win.

Comment 21

[cdrevland]

A truly excellent simplicity win on both Windows and OSX! It's perfect. When will it be checked in?

Comment 22

[Matthias Versen [:Matti]]

I often use reloading on a page that is loads very slow and one button is very confusion in safari, one of the reasons i don't use safari....
But nobody could complain if you have the option to have 2 separate buttons.

Comment 23

[Dão Gottwald [:dao]]

that option would be there with the attached patch.

Comment 24

[Biju]

Another thought "Activity Indicator" and "Stop Button" both active and inactive at same time so why do we have both of them?

Comment 25

[David Regev]

I have a bug report and three or so ideas for improvement. I apologize ahead of time for the length. First, the bug: the Stop-Reload transition is not tab-specific. Steps to reproduce:

1. Load a page in a new tab.
2. Quickly switch tabs (before the page finishes loading).
Expected results: Stop changes immediately to Reload.
Actual results:   Stop persists until the first tab completes loading.

Now, the ideas. With the standard implementation of the Stop-Reload toggle, one major problem is what happens when trying to stop a page load (which this bug attempts to solve). When a user wants to stop a page from loading, the following sequence ensues:

I. Wonder if the page load has completed / make sure that it hasn't.
II. Navigate the cursor over to Stop:
    a. first, by mentally resolving to do so,
    b. then, by physically doing so.
II. Verify that the page hasn't completed loading, so as not to click Reload.
IV. Click.

Note that there are two steps here that have been added by the toggle: I and III. With the normal two-button system, these steps are unnecessary, as the the Stop button will never disappear, and, thus, there is no penalty to activating it after a page load. Ideally, both these steps would be eliminated.

As for step I, this can be eliminated by making sure that the user already knows subconsciously that the page hasn't completed loading. If a specific sound is always played when a page completes loading, then the user will never have to wonder if the page has loaded. (Bug 265037 would fix this on Windows.)

As for step III, this is, of course, what this bug fixes. There will be no need to pause and make sure Stop is still active if there is no chance Reload will be enabled when it's time for step IV. However, I fear the current delay (500 ms) may be too short. Once step II has begun, which is before any actual physical movement of the cursor, the user will have started one full gesture, and will not notice any changes in the environment until that gesture is complete. Thus, the delay has to be at least as long as the length of all of step II (together with step IV) for most situations and for most users. If a user accidentally hits Reload a few times, the browser will not be trusted again, and the user will subconsciously perform step III from now on. That is why I think it is better to err on the side of too long rather than too short. Thus, increasing the delay, e.g., to 1000 ms, would be good. Having an about:config preference for testers to tweak to help figure out the ideal value would also be useful.

The longer that transition, however, the greater the chance of the user sometimes having to wait too long in order to click Reload. Thus, when the user wants to reload a page, there will sometimes be a new step:

V. Wait for Reload to become active.

My solution to eliminate that wait is to make clicks on the disabled Stop button end the transition and show the active Reload button immediately. It might also be useful to make the button appear as clickable on hover (but still using the disabled Stop icon state) to increase discoverability of this feature.

These ideas should, theoretically, remove the extra steps and bring the toggle back to the efficiency of the standard two-button system. At least, that's the plan. What do you think?

Comment 26

[Grey Nicholson (they/them)]

(In reply to comment #15)
> Created an attachment (id=302443) [details]
> v4 (extension)

When the button changes from Stop to Reload (for example, when switching tabs), there's a short delay during which the button is a disabled Stop button. But when changing from Reload to Stop, there's no such delay.

Should the delays be symmetrical? Or is there some case where it's very important to be able to stop a page reload within 500 ms of starting it?

Comment 27

[Ben Basson]

Just had a quick play with this extension and compared to Safari.

I find the 500ms delay between a disabled stop button and showing the refresh button visually noticeable every time, mainly because there's a transition from colour to grey back to colour (and it's a different colour).

Safari has an instant changeover, which I think we can all agree isn't perfect, but its buttons are also all the same colour and I think that really helps the behaviour to not be annoying.

It might be better if the intermediate "disabled" step were skipped and the button simply stayed on "stop" for a little longer. I think 500ms is a bit excessive though. 

Comment 28

[David Regev]

Ben, that would only serve to delay the accidental reload issue. It would also be misleading, as Stop would be active when there is nothing to stop.

Comment 29

[cdrevland]

I really hope this will be implemented. At the very least the extension should be updated and added to addons.mozilla.org when Fx3 is out. This is exactly how it should be.

Comment 30

[Andreas M.]

I also vote for changing this. It seems unlogical to have a user-interface element, that does nothing 90% of all the time sitting at such a prominent place like the main-toolbar. It can not be used, it is even ghosted. It is not possible to cancel loading of a page, that has already been loaded. This is unintelligent user-interface design.

Opera does it the right way. Yet their users seem not to have complained either, otherwise they would have changed the behaviour.

Also, for people with 1024x768 (many Laptops) it takes away precious screen estate, that could be used by other buttons.

I was expecting FF3 to have this as default. Or at least as a user-preference. I do not like to install extensions for such little things.

Please, could you make this behaviour a user-option? Thanks.

Comment 31

[Dão Gottwald [:dao]]

Attachment: v5 (extension)

Comment 32

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

Comment on attachment 329292 [details]
v5 (extension)

I can only do ui-reviews in cases where there is a very high likelihood that the drivers will agree with my decision.  I know this control scheme has been debated a lot in the past, so I need to set the review back to beltzner for his feedback

Comment 33

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

Jumping to a slightly higher level, here is what I currently think the ideal control scheme for loading is:

-Similar to Chrome, merge go/stop, so the stop button is at the end of the location bar (I would move the star to left side, but inside of the bar to the right of the site button).  I really like how the flashing control is usually outside of your peripheral vision.  I also like how reload, which is a commonly used control, remains stable and doesn't change state or purpose while loading.

-add a progressbar inside of the location bar similar to Safari, but I would make the progress bar only 1 or 2 pixels tall, and running along the bottom of the text field.

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 329292 [details]
v5 (extension)

Alex: Beltzner won't look at this bug spontaneously, and I can't convince him either. So this bug would just continue to rot away. I don't expect you to make a decision, but my idea is that you as part of the UE team talk to him, and if you can convince him, grant ui-r.

This is pretty conservative and straightforward implementation-wise, continues to allow customization and it doesn't have bad impact on third-party themes. Whether we should eventually merge Stop and Go or do other restructuring, I don't know. But I'd like to make progress rather sooner than later and reach agreement that this is the next best thing to do (e.g. better than bug 462640).

Comment 35

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

>I don't expect you to make
>a decision, but my idea is that you as part of the UE team talk to him, and if
>you can convince him, grant ui-r.

Ok, will do

Comment 36

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

Comment on attachment 329292 [details]
v5 (extension)

I had a chance to talk to beltzner about this before heading off for vacation.  We unfortunately can't change the UI in any significant way this close to release.  Even a few considerably more minor modifications (like the drop down next to the navigation controls being removed on OS X), haven't been able to land since we want more time for feedback.  So removing myself as a reviewer purely due to the time constraints, as opposed to giving a +/-

Comment 37

[Dão Gottwald [:dao]]

Comment on attachment 329292 [details]
v5 (extension)

No problem - I abandoned my hope that this could make it into 3.1 long ago. 3.2 seems like the right target now, with a lot of time for feedback.

Comment 38

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

Comment on attachment 329292 [details]
v5 (extension)

Looking forward to reviewing this when the change is possible again, but in the meantime trying to keep my review count at "zarro"

Comment 39

[Dão Gottwald [:dao]]

I don't understand what "when the change is possible again" means. It's possible right now on trunk.

Comment 40

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

Good point, my mind is still focused on 3.1 stuff, but yeah, you're right.

One sort of related dependency: having the ability to place multiple versions of the same control in the customization palette would allow us to ship this regardless of if it is in the default set.

Comment 41

[Dão Gottwald [:dao]]

What the current implementation does is to just merge the two buttons when they are placed next to each other on the toolbar.

Comment 42

[Mike]

Given the number of downloads and responses about the extension, it makes a lot of sense to combine them.

And to keep the crowd that prefer the classic look a simple check box under "customize toolbar" window or possible somewhere in options (possibly under accessibility) should be sufficient.

Doing this does 2 things: it simplifies the interface, and allows users to customize the browser, or revert the changes.  Most people I think would prefer a single button.

just my 2 cents

Comment 43

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

This bug's priority relative to the set of other polish bugs is:
P1 - Polish issue that appears in the main window, or is something that the user may encounter several times a day.

We may make a different decision on how exactly we want to combine or change the interface of stop and reload, but the overall issue is definitely P1 since the controls are commonly used and this is in the main window.

Comment 44

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

(switching to whiteboard for polish-relative priorities)

Comment 45

[Taylor Braun-Jones]

As an advocate of this enhancement, I wanted to offer my $0.02: Most average computer users I know who install Google Chrome say the thing they most like about it is how simple the interface is. I think the combined stop/reload button is part of that.

Comment 46

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

Some mockups of a possible stop / reload / go implementation for Firefox 4 are here: https://wiki.mozilla.org/Firefox/4.0_Windows_Theme_Mockups#Combo_Stop.2FRefresh.2FGo_Button.

Comment 47

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

Comment on attachment 329292 [details]
v5 (extension)

This works well, but I think the only thing missing before we are ready to land this on trunk is to have three controls listed in the customization panel:

-Reload and Stop
-Reload
-Stop

And to remove the behavior of grouping into one button if they are placed next to each other, since this may be confusing in the situation where the user wants separate controls, but also wants them to be next to each other.

Comment 48

[Dão Gottwald [:dao]]

Attachment: v6

I'm not sure that confronting users with slight variations of the same controls is the ideal solution. Also, we don't have a decent way to migrate profiles from [stop][reload] to [stop-and-reload]. So this version keeps the auto-merging but adds a "Combine Stop and Reload" checkbox to the customization panel.

Comment 49

[David Regev]

(In reply to comment #48)
> I'm not sure that confronting users with slight variations of the same
> controls is the ideal solution. Also, we don't have a decent way to migrate
> profiles from [stop][reload] to [stop-and-reload]. So this version keeps the
> auto-merging but adds a "Combine Stop and Reload" checkbox to the
> customization panel.

A possible solution is to add a ‘No-width Space’ to the the customization panel. If a user wanted to prevent two controls from merging, the no-width space can be dragged in-between them. This avoids the need for separate variations of the same buttons or for checkboxes, and it solves the migration issue. A nice benefit of this solution is that it can be re-used for several other buttons that may be merged in the future: Stop/Reload and Go, Location bar and Search bar, perhaps even Title bar and Tab bar. (This also makes toolbar buttons work a lot like ligatures, which is kind of cool.) Just a thought.

Dão: I’ve noticed with your extension that the button sometimes gets stuck on the intermediate state. I was never able to figure out the cause, but someone commented[1] on your site that it may happen when the page hasn’t loaded properly. Is this fixed in your latest version?

Also, as I argued in comment #25, I think the intermediate state should be clickable and clicking on it should end the delay and show Reload immediately, as if the page-load was stopped. I have found that this delay becomes noticeable if I want to reload the page for some reason while it is already loading and happen to hit the button during that state. Had I reached it slightly earlier, I would have been able to double-click on the button to stop and reload the page. Because the transition isn’t clickable, I need to wait between clicks.

[1] http://en.design-noir.de/mozilla/stop-reload/?com#c:26

Comment 50

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

I think a direct manipulation approach of dragging the exact controls you want out of the customization pallet is the most straightforward.  To reduce confusion over some of the controls looking the same we could change the icon for the combined stop reload to sort of cut in half and show half of each icon or something.

Comment 51

[Mike Connor [:mconnor]]

I agree that it's likely better, but I don't think it's absolutely required in this case.  There's some implications with addon compatibility to adding multiple copies of the same control.

I think v5 + comment 49 would be a good thing to try, the checkbox is  unnecessary, IMO.

Comment 52

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

>I agree that it's likely better, but I don't think it's absolutely required in
>this case.  There's some implications with addon compatibility to adding
>multiple copies of the same control.

Then how about we go with this approach for 3.7 and break the addons for 4.0?  There are a number of things we need to get figured out to clean up the UI for customization of controls, and a few cases where we may need to have multiple forms of the same control (for instance, home, new tab, back/forward, etc.)

Comment 53

[Dão Gottwald [:dao]]

Sure, this could be revisited later, although I don't see why we would need multiple versions of home and new tab -- they just need different styling depending on where they are located, something that we could do today.

Comment 54

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

>they just need different styling
>depending on where they are located, something that we could do today.

My impression was that the new tab button needed to be different controls behind the scenes to move between the tab strip and toolbar, but I'm likely confused.  Either way we need to support customization of new tab and home.

Comment 55

[Dão Gottwald [:dao]]

I assumed that the tab strip would be a toolbar by then. (Currently it doesn't participate in toolbar customization at all, so multiple versions of these controls wouldn't help.)

Comment 56

[Mike Connor [:mconnor]]

Yeah, breaking compat for 4.0 is fine, we'll break a lot of stuff with 4.0's possible UI changes, so that's the right time to re-evaluate toolbar customization more holistically.

Dao: can you get an update to the extension with v5 + the tweak from comment 49?  I think we're close to a solution here.

Comment 57

[Dão Gottwald [:dao]]

Attachment: v5.1

with that change (clicking stop while it's disabled switches to reload)

Comment 58

[Dão Gottwald [:dao]]

Attachment: patch

Comment 59

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

>with that change (clicking stop while it's disabled switches to reload)

I'm worried about users who don't differentiate between single clicks and double clicks while they use their computer.  Also, I'm worried that this population might be significantly larger than the set of people who need to stop and reload before the timer on the control.

For instance, had we un-bookmarked a page on the second click of the star in the location bar, a lot of users would double click to bookmark, and never be able to create a bookmark (google hit this problem with a massive number of google toolbar users).

Comment 60

[Dão Gottwald [:dao]]

Comment on attachment 329292 [details]
v5 (extension)

That brings us back to this version. (I'm intentionally not canceling the other request, so that you can pick the preferred version.)

Comment 61

[David Regev]

(In reply to comment #59)
> >with that change (clicking stop while it's disabled switches to reload)
> 
> I'm worried about users who don't differentiate between single clicks and
> double clicks while they use their computer.  Also, I'm worried that this
> population might be significantly larger than the set of people who need to
> stop and reload before the timer on the control.

This would be a problem for all three states of the combined button. Those users who double-click everything will find that their attempts at reloading have failed. Similarly, they will find that their attempts at stopping a load have actually triggered a reload. The former of these scenarios is far more likely for these users than accidentally double-clicking on the transition state. I suspect those users might quickly learn. I also suspect that the intersection of users who double-click everything and users who actually stop page loads is insignificant, which means the the latter scenario may not be that common.

In any case, there is a good argument for capturing accidental double-clicks on the Reload state, especially since reloading and immediately stopping is not a common use case (as far as I know).

Comment 62

[Dão Gottwald [:dao]]

Attachment: patch v5.1

Comment 63

[Dão Gottwald [:dao]]

Attachment: patch v5

Comment 64

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

>This would be a problem for all three states of the combined button. Those
>users who double-click everything will find that their attempts at reloading
>have failed. Similarly, they will find that their attempts at stopping a load
>have actually triggered a reload. The former of these scenarios is far more
>likely for these users than accidentally double-clicking on the transition
>state.

Yeah, you're right.  In addition to not processing double clicks (or really any clicks) in the transition state I wonder if we need a very slight (and not visible) timer so that a double click will actually reload, or stop.  Either way, we can deal with that in a follow up bug so that we can finally land this.

Comment 65

[Dão Gottwald [:dao]]

Comment on attachment 402815 [details] [diff] [review]
patch v5

pretty straightforward and shouldn't take too long to review -- please redirect if you absolutely don't have time.

Comment 66

[Dão Gottwald [:dao]]

Attachment: patch v5.0.1

This fixes the enabled-but-not-clickable stop button, which wasn't caused by my other changes but became increasingly annoying here because it blocked reloading. I've seen this frequently on iGoogle. I haven't yet noticed any weirdness with this patch, but I'm not sure if we should also require STATE_IS_NETWORK for STATE_STOP.

Comment 67

[David Regev]

(In reply to comment #66)
> This fixes the enabled-but-not-clickable stop button, which wasn't caused by
> my other changes but became increasingly annoying here because it blocked
> reloading. I've seen this frequently on iGoogle.

Great! I was never able to reproduce that bug consistently. What was its cause?

This reminds me of another issue. On sites like iGoogle, Stop can deactivate and reactivate several times while the page (with all its frames) is loading. Beforehand, this was just a Firefox quirk. With a combined button, however, the button can cycle through the Stop, Intermediate, and Reload states a few times before the page is fully loaded. That is a bit confusing and also makes stopping the page fully somewhat tricky. Ideally, it should remain on the Stop state until the page is actually done loading. This is more of a separate bug and might be difficult to fix, but I thought I’d bring it up, since it affects the combined Stop/Reload especially.

Incidentally, is the disabled Stop going to be clickable in the end or was that change backed out?

Comment 68

[Dão Gottwald [:dao]]

(In reply to comment #67)
> (In reply to comment #66)
> > This fixes the enabled-but-not-clickable stop button, which wasn't caused by
> > my other changes but became increasingly annoying here because it blocked
> > reloading. I've seen this frequently on iGoogle.
> 
> Great! I was never able to reproduce that bug consistently. What was its cause?

We started our activity indicators, enabled stop etc. when we got a state change notification with the STATE_START flag set without making sure the STATE_IS_NETWORK flag is set too. See https://developer.mozilla.org/en/nsIWebProgressListener#State_Type_Flags for what these flags mean.
A comment suggests that this was done on purpose, but apparently it broke at some point if it ever worked correctly.

> Ideally, it should remain on the Stop
> state until the page is actually done loading.

The page is actually done loading when this happens, but it started subsequent requests.

Comment 69

[Dão Gottwald [:dao]]

Attachment: patch v5.0.1

updated to latest trunk

Comment 70

[Dão Gottwald [:dao]]

(In reply to comment #66)
> I haven't yet noticed any weirdness with this patch

... and still haven't, by the way. I've been continuously using it ever since.

Comment 71

[Mardeg]

Can someone link to a tryserver build of this for more testing please?

Comment 72

[Dão Gottwald [:dao]]

https://build.mozilla.org/tryserver-builds/[e-mail]-try-bc7f1b9418c6/

Comment 73

[Emil Ivanov]

With this build it looks like the stop-reload button is updated slowly from stop to reload after the throbber finish. Its like 1-2 seconds lag.
I dont like the disabled state, before reload button change. Maybe this is giving me the impression for the "lag".

IMO it will be better if the mouse is over the active stop button to make it disabled for a second and then show reload button, else just show the reload button.

Comment 74

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

I heard that Chrome was using a hover-based approach, so if you hover into the button when it is on the stop state, it stays as a stop button until you hover out (stopping a finished page of course does nothing).  This might end up working better than a timer, what do people think?

Comment 75

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

(Of course they are combining stop/go, but the hover idea could still apply)

Comment 76

[David Regev]

(In reply to comment #74)
> This might end up working better than a timer, what do people think?

That’s very clever. Wouldn’t it be confusing, though, if the page has stopped loading but the Stop button is still active? It would convey the page’s status more effectively if the disabled Stop state is what remained on hover, not the enabled Stop.

In any case, the hover trick without the timer wouldn’t fully solve the problem: if I start moving the pointer to the Stop button and the page completes loading _right before_ I get there, I will still accidentally hit the Reload button. If, however, there is an intermediate state, I will simply hit the disabled Stop button.

(In reply to comment #73)
> With this build it looks like the stop-reload button is updated slowly from
> stop to reload after the throbber finish. Its like 1-2 seconds lag.
> I dont like the disabled state, before reload button change. Maybe this is
> giving me the impression for the "lag".

This comment is likely not to be unique. Many people won’t get the intermediate state, and will assume that something is broken or just very slow. Perhaps some thought should be put into making the intermediate state look more obvious as a real and necessary state. (I, for one, have suggested[*] something like a yellow traffic light––a metaphor people should immediately recognize.)

[*] https://wiki.mozilla.org/Talk:Firefox/4.0_Windows_Theme_Mockups#Merged_Stop.2FReload.2FGo_Button

Comment 77

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

>In any case, the hover trick without the timer wouldn’t fully solve the
>problem: if I start moving the pointer to the Stop button and the page
>completes loading _right before_ I get there, I will still accidentally hit the
>Reload button.

Yeah, that's a good point, I wonder if average reaction time is fast enough to see the change while you mouse into the target.  The user should have their visual gaze focused on the target as they move the mouse cursor in.  Something we could play with if we wanted to get even fancier is potentially creating a larger hover area (not sure if this is technically feasible though).

Comment 78

[Mike Connor [:mconnor]]

Comment on attachment 405042 [details] [diff] [review]
patch v5.0.1

Sorry for the entirely-too-long delay here.

I think we should get this landed ASAP, and then iterate as needed in new bugs.

Comment 79

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/613b4bf6bc6f

Comment 80

[Boris Zbarsky [:bzbarsky]]

Regression: Txul increase 1.75% on Linux Firefox
Previous results: 110.895 from build 20091217074829 of revision 1eb1763c6540 at 2009-12-17 08:05:00 on talos-rev2-linux01 run # 0
New results: 112.833 from build 20091217085134 of revision 613b4bf6bc6f at 2009-12-17 09:09:00 on talos-rev2-linux04 run # 0

Suspected checkin range: from revision 1eb1763c6540 to revision 613b4bf6bc6f

Comment 81

[Dão Gottwald [:dao]]

(In reply to comment #80)
It's somewhat expected that modifying the toolbar when opening a window affects Txul. I don't see how to mitigate or solve this. So I'd like Connor to weigh in as to whether this is acceptable.

Comment 82

[Boris Zbarsky [:bzbarsky]]

It's not that expected that the operations you do here would affect Txul by 1%.  That seems like a lot of time to me (2ms on that box, to be precise).  Does the wrap() method really take that long?  If so, why?  If not, what _is_ taking the time?

Comment 83

[Mike Connor [:mconnor]]

This is too much.  Need to figure out if there's a better way of doing this.  ~2% is really high... Please back this out for now.

Comment 84

[d]

Will this only be removed from Linux, or does this apply to all OS's?

Comment 85

[Mike Connor [:mconnor]]

We'll back out the whole patch, and fix it.  I suspect there's other regressions lurking, since those are usually reasonably consistent.

Comment 86

[Dão Gottwald [:dao]]

Comment on attachment 405042 [details] [diff] [review]
patch v5.0.1

>+    stop.parentNode.replaceChild(this.container, stop);
>+    this.container.appendChild(stop);
>+    this.container.appendChild(reload);

This is where the time is spent.

Comment 87

[Dão Gottwald [:dao]]

Attachment: patch v6

This avoids the issue by not wrapping the buttons. This means that other elements would be shifted if stop and reload had different widths, so the merger is limited to icons mode.

Comment 88

[Sk]

Dear developers, please reconsider closing this bug. You try to please those three-percenters using chrome, but don't consider your core-base audience habits.

Comment 89

[Taylor Braun-Jones]

What study did the three percent statistic come from?

Comment 90

[Sk]

This for example: http://news.softpedia.com/news/Chrome-Usage-Sees-Healthy-Growth-in-October-125965.shtml
But it doesn't really matter, chrome user-base is way less then firefox's one.
And trying to make chrome users like firefox UI more, but not listening to old firefox users is a bad strategy, in my opinion. Time will tell.

Comment 91

[d]

I know this isn't the place for a long discussion, but this isn't about making Firefox look like Chrome. This is about saving room in the toolbar, and as it happens, Chrome was just ahead on implementing this. As you can see, it was actually suggested first here, in 2006. I see no negative aspects of this change, I never use the abort button anyways, but for the people who won't like this change, do we offer two separate buttons too? (Hidden inside the options, of course)

Comment 92

[Boris Zbarsky [:bzbarsky]]

> This is where the time is spent.

That should absolutely not take 2ms.  I assume you can reproduce that locally?  Is it happening just on Linux for you, or on all platforms?  If it happens on Mac, I'm happy to try profiling it....

Vsevolod, Taylor, Magne, can you please take the advocacy and meta-discussion somewhere where it won't interfere with the work on pinning down the performance problem?

Comment 93

[Dão Gottwald [:dao]]

(In reply to comment #92)
> That should absolutely not take 2ms.  I assume you can reproduce that locally? 
> Is it happening just on Linux for you, or on all platforms?  If it happens on
> Mac, I'm happy to try profiling it....

I can reproduce it locally on Linux, haven't looked elsewhere. But http://graphs.mozilla.org/dashboard/?tree=Firefox shows that the regression was cross-platform, so I think you should see the same on Mac.

Comment 94

[Boris Zbarsky [:bzbarsky]]

OK.  Over here, 100 wrap()/unwrap() call pairs take about 118ms.  Looking into why now.

Comment 95

[Boris Zbarsky [:bzbarsky]]

Alright.  So removing the stop/reload buttons from the document has to unhook their XBL bindings, which tears down the anonymous content, which unhooks some XBL bindings attached to parts of that anonymous content.  It also has to blow away the boxes that were created for all this stuff.  This causes the removal to take about 20x as long as just removing a node with no XBL and no presentation takes.

Then inserting the nodes has to reattach the XBL bindings, recreate the anonymous content, and rebuild the presentation for all those nodes.

In general terms, 25% of the time is spent instantiating XBL bindings, 10% of the time is spent on selector matching and creating style contexts, 10% tearing down XBL bindings, 5% removing frames, 15% creating frames, 10% what looks like setting properties that run a bunch of xbl-installed js and then cause it to set DOM attributes.  Treat all these with a grain of salt, since this is on a baseline of 80 profiler samples for the single wrap() call (didn't want to pollute the profiler data with whatever unwrap is doing).

Obvious question: is it feasible to run this code before StartLayout has created the frame tree and instantiated all that XBL?  And if so, will it still work right if run at such a point?

Next obvious question: how many elements are actually involved in the stop/reload button bindings, and why are there multiple elements there to start with?

Next obvious question: can we somehow speed up this XBL junk in general?

Comment 96

[Boris Zbarsky [:bzbarsky]]

Note: it looks like XUL documents guarantee that DOMContentLoaded and onload will fire after StartLayout.  I see no reasons not to switch that up so that we fire DOMContentLoaded _before_ StartLayout and onload after, if that would help.  Are there BrowserStartup operations that would be possible, and cheaper, before the xbl bindings and frames are all set up?

Comment 97

[Dão Gottwald [:dao]]

(In reply to comment #95)
> Obvious question: is it feasible to run this code before StartLayout has
> created the frame tree and instantiated all that XBL?  And if so, will it still
> work right if run at such a point?

I don't think so... We depend on the <toolbar> constructor.

> Next obvious question: how many elements are actually involved in the
> stop/reload button bindings, and why are there multiple elements there to start
> with?

There's a label and an image, for the icons, text and icons+text modes.

(In reply to comment #96)
> Note: it looks like XUL documents guarantee that DOMContentLoaded and onload
> will fire after StartLayout.  I see no reasons not to switch that up so that we
> fire DOMContentLoaded _before_ StartLayout and onload after, if that would
> help.  Are there BrowserStartup operations that would be possible, and cheaper,
> before the xbl bindings and frames are all set up?

There's a lot of stuff that depends on the tabbrowser binding, for instance. Ignoring this, I don't think there's much to win.

Comment 98

[Boris Zbarsky [:bzbarsky]]

> There's a label and an image,

Oh, and the label has its own XBL binding, with more nodes.... ok.  :(

Comment 99

[Dão Gottwald [:dao]]

It has its own binding, but I don't think it spawns more nodes.

Comment 100

[Mike Connor [:mconnor]]

Comment on attachment 418343 [details] [diff] [review]
patch v6

Looks good, sorry for the vacation-induced delay.

Comment 101

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/8b2c716b1532

Comment 102

[IU]

There's an oversight with the patch: If the stop button precedes the reload button: *DOES NOT MERGE*

Comment 103

[Emil Ivanov]

(In reply to comment #102)
> There's an oversight with the patch: If the stop button precedes the reload
> button: *DOES NOT MERGE*

Thats for those who dont want to use them as single button.

Comment 104

[Emil Ivanov]

Changing the summary per comment 102 to avoid confusion.

Comment 105

[Jim Jeffery not reading bug-mail 1/2/11]

OK, once they are 'merged' how do you 'un-merge' the single button and go back to separate stop/reload ?

Comment 106

[Soothsayer]

As well as a 'me to' on the request on how to undo this whilst keeping the stop and reload buttons next to each other, this change appears to have caused a regression.

If you separate the buttons to prevent them merging, the stop button is now always enabled and no longer has a disabled state.

Comment 107

[u88484]

(In reply to comment #106)
> If you separate the buttons to prevent them merging, the stop button is now
> always enabled and no longer has a disabled state.
I don't see this on Windows 7

Comment 108

[Soothsayer]

Thanks for checking Kurt.

Oddly, now I have restarted Minefield again, I am now unable to reproduce this issue.

I will leave it for now and if I manage to come up with a reproducible example, will file a new bug.

Comment 109

[Bill Gianopoulos [:WG9s]]

I have posted this elsewhere, but thought perhaps people would be more likely to find it in the bug.

If you simply must have the old layout with the old button order, you can insert a flexible space between the reload and stop buttons.  Since the locationbar expands in size to use all the unused space in the toolbar, the flexible space will collapse to zero width unless you do not have the stop and reload buttons on t\he same toolbar as the location bar.

Comment 110

[Alfred Kayser]

To prevent the merging of the two buttons, just switch them around, so that they are stop | reload. 
So for the SUMO page on this:
If you don't want the reload and stop button merge into one button, 
or if you want to keep the buttons separate:
Go to View/Toolbars/Customize, 
and drag the reload button to the other side of the stop button,
and click done.

Other way round, if you want the buttons merge, but it doesn't work any more, check that the stop button is directly after the reload button (with nothing in between).