GNOME Bugzilla – Bug 657655
[WEB] Combine the Stop and Reload buttons into one
Last modified: 2011-12-20 00:34:53 UTC
So that epiphany becomes a modern browser at last.
Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find. *** This bug has been marked as a duplicate of bug 165137 ***
Created attachment 195123 [details] [review] egg-toolbars-model: add a method to retrieve the locations of an item
Created attachment 195124 [details] [review] toolbars: Combine the Reload and Stop menu actions into one Make this combined menu action the default for the toolbars. This transforms epiphany in a modern browser.
Created attachment 195125 [details] [review] toolbars: remove the Stop and Reload menu actions Also, make sure that toolbars including them are migrated to use the new combined action, by replacing all the Reload items with the Combined version. If no Reload item is found in a toolbar, replace then the Stop items, otherwise just get rid of them.
I dare to reopen, in the light of the patches attached.
I noticed that this can be simplified, because menuitems can be only in one location, so there's no need to deal with lists. But I think this can be fixed after merging, so that we don't miss the UI freeze.
Review of attachment 195125 [details] [review]: I think this is the wrong way to do it. We should do a one-time migration using the existing migration mechanism that edits the XML files. Constantly replacing the obsolete items forever seems a bit pointless.
Review of attachment 195124 [details] [review]: This looks good to me! ::: src/ephy-toolbar.c @@ +602,3 @@ + EphyToolbarPrivate *priv = toolbar->priv; + + if (is_loading) { I think the braces are wrong here (sorry, I know this is a mess).
Review of attachment 195123 [details] [review]: I guess whether this goes in or not depends on how we do the migration :)
(In reply to comment #5) > I dare to reopen, in the light of the patches attached. Patches or not, I still think this is a bad idea. The arguments from bug 165137 still stand.
*** Bug 660341 has been marked as a duplicate of this bug. ***
(In reply to comment #10) > (In reply to comment #5) > > I dare to reopen, in the light of the patches attached. > > Patches or not, I still think this is a bad idea. The arguments from bug 165137 > still stand. The one about confusing users? iOS, Safari, Chrome and Firefox all have a combined stop/refresh button. I don't think that your comment from 6 years ago still holds. The point about a11y doesn't really hold either, Totem has had a combined play/pause button for a long while without causing damage.
I understand that a redesign is underway ( http://youtu.be/b2VALT-DMGU ) and if the toolbar is going to disappear then this bug is obsolete. Still, as long as we have a Stop button I would expect it to stop animations and scripts within a webpage when not in page loading state, instead of turning into a Reload button. As for accessibility, I propose we leave that for the experts to comment on. But I do know that a web browser is not the same thing as a media player where Play and Pause are two mutually exclusive states. :)
(In reply to comment #13) > Still, as long as we have a Stop button I would expect it to stop animations > and scripts within a webpage when not in page loading state. Currently, when not in loading state, the stop button is grayed out. So your expectations are not fulfilled either.
Created attachment 203590 [details] [review] simple approach This is the simplest approach, a bit hackish, since it involves tweaking the action in EphyWindow. Not my preferred one.
Created attachment 203591 [details] [review] Combine the Reload and Stop toolbar buttons into one Add a combined action that toggles itself between the two states and instantiate it from the window. Replace the Reload and Stop buttons with the combined one in the default toolbar description.
The second patch would be my preferred one.
(In reply to comment #13) > as long as we have a Stop button I would expect it to stop animations > and scripts within a webpage when not in page loading state Even with Stop and Reload combined, being able to stop page loading and animations still is a needed functionality. With Firefox you can get that by pressing the Esc key Maybe a new bug report should be filled to allow user to stop animations and so on ?
I believe you're mistaken. The stop buttons of all browsers I know only stop the load process, not animations, including Firefox'es. For instance, how do you stop this clock from ticking, in Firefox? http://css-tricks.com/examples/CSS3Clock/
(In reply to comment #19) > I believe you're mistaken. The stop buttons of all browsers I know only stop > the load process, not animations, including Firefox'es. For instance, how do > you stop this clock from ticking, in Firefox? > > http://css-tricks.com/examples/CSS3Clock/ I can't stop the clock from ticking by pressing Esc with Firefox. However I can stop animated gif that way
Please stop arguing here about an unrelated feature and just file a bug if you care, thank you.
Review of attachment 203591 [details] [review]: I think I would name the action combined-reload-stop instead of just combined, otherwise it could be a combination of pretty much anything... Otherwise it looks pretty good to me. ::: src/ephy-combined-action.c @@ +50,3 @@ + +typedef enum { + EPHY_COMBINED_ACTION_STOP = 0, The = 0 is not really needed. @@ +72,3 @@ + EPHY_COMBINED_ACTION_STOP : EPHY_COMBINED_ACTION_REFRESH; + prev_action_enum = action->priv->loading ? + EPHY_COMBINED_ACTION_STOP : EPHY_COMBINED_ACTION_REFRESH; You can just flip the boolean here right? There's only two options. @@ +80,3 @@ + NULL); + + g_signal_handlers_disconnect_by_func (action, I think it would probably be cleaner to just store the handler ID and uninstall it if it exists, instead of doing this?
Created attachment 203734 [details] [review] Combine the Reload and Stop toolbar buttons into one The name is legacy -- I was intending to make it a generic combined action, but then I realized that it was a bit overkill since we don't plan to use anything similar. The patch is updated with your suggestions, the class has been renamed as well. Add a combined action that toggles itself between the two states and instantiate it from the window. Replace the Reload and Stop buttons with the combined one in the default toolbar description.
Review of attachment 203734 [details] [review]: Let's do this.
Attachment 203734 [details] pushed as b1cd685 - Combine the Reload and Stop toolbar buttons into one
(In reply to comment #21) > Please stop arguing here about an unrelated feature and just file a bug if you > care, thank you. The Stop button has been stopping animations since the early days of Netscape! You're perfectly entitled to your opinion that this use case isn't enough to warrant keeping a Stop button around when the page is loaded but please don't act condescending like we're asking for some outlandish feature. Thank you.
I just don't understand your stance here. I've only said that neither the Stop nor the Esc key were used for stopping animations before we decided to make this change so neither seem like a sound argument to be against it (not to mention that 'Esc' as an option doesn't seem to have any relationship whatsoever with the presence or lack of a Stop button) and that in case you want either of these features available you should file a new bug report, where we and anyone interested can discuss freely on the merits of such feature without adding noise to unrelated ongoing discussions.
(In reply to comment #27) > I just don't understand your stance here. I've only said that neither the Stop > nor the Esc key were used for stopping animations I'm pretty sure that they were in Epiphany when we were still on a Gecko back-end. I've always thought of this as a regression that would be fixed one day. > before we decided to make > this change so neither seem like a sound argument to be against it (not to > mention that 'Esc' as an option doesn't seem to have any relationship > whatsoever with the presence or lack of a Stop button) The relationship follows from the fact that the Esc key is/was a keyboard shortcut for the Stop button.