Pressing Esc does not stop page load

[benzea]

In Browse 104 I cannot press Escape to cancel the page load. Pressing the stop button works fine though.

[tomeu]

Looks like a missing accelerator.

[RafaelOrtiz]

Confirmed, moving to 0.94.

[manuq]

Patch looks good, would be nice to ask for design approval, and if there's positive feedback, push it in the next cycle (0.98).

[garycmartin]

Just tested in Safari, Firefox, and Chrome, they all use the esc key to stop a page load so seems an OK additional keybinding to add to Browse. The only potential conflict in Sugar is that esc is used to exit from fullscreen mode in Activities that support it (e.g. Turtle Art, Image Viewer). If Browse adds a fullscreen feature as some point in the future, esc would need some extra logic. Something like 1) if in fullscreen mode and a page is not loading, exit fullscreen mode; 2) if in fullscreen mode and a page is loading, stop loading -- this is how Safari handles the esc case.

[humitos]

manuq or gonzalo, can you re-consider applying this patch?

[godiard]

Replying to humitos:

manuq or gonzalo, can you re-consider applying this patch?

How you solve the conflict with the exit of fullscreen mode (see previous comment by ary)

[humitos]

Replying to garycmartin:

Something like 1) if in fullscreen mode and a page is not loading, exit fullscreen mode; 2) if in fullscreen mode and a page is loading, stop loading -- this is how Safari handles the esc case.

Well, this is not so easy to do because in that case we have to modify the Activity class to handle this situation. In fact, I think it's not a good idea to modify the Activity class just for this special situation. Maybe we can think in something more general like defining a "state" (boolean) variable to add in the if clause.

At the moment, Sugar's Activity class take priority over the "key-press" event (sugar3/graphics/window.py L250). So, if we are loading a web page in fullscreen and we press Esc, we will be "unfullscreen" and the web page will keep loading.

[manuq]

Replying to humitos:

Replying to garycmartin:

Something like 1) if in fullscreen mode and a page is not loading, exit fullscreen mode; 2) if in fullscreen mode and a page is loading, stop loading -- this is how Safari handles the esc case.

Well, this is not so easy to do because in that case we have to modify the Activity class to handle this situation. In fact, I think it's not a good idea to modify the Activity class just for this special situation. Maybe we can think in something more general like defining a "state" (boolean) variable to add in the if clause.

Remember classes and inheritance? :P You can redefine the callback (as we did in Terminal some months ago #440) Just remember to call the parent's callback. Is a bit hackish because is a private method and have the name mangling with the two underscores, but I think doable.

[humitos]

v2 - considers full screen mode

[manuq]

Thanks! Pushed as 41968d61 .

[erikos]

add overwritable escape method

[erikos]

use overwrite method

[erikos]

As said in ​http://bugs.sugarlabs.org/ticket/4013#comment:8 I am not happy with the approach of these hacks. I would do it more in the way as in the patches attached above ^.

However I wonder if we should handle that case. When I am in fullscreen mode and a page does load that I want to stop I do have to be in laptop mode to use the escape key, ebook mode will not work. Furthermore if the page did not stop I have an unloaded state. What can I do now? Reload the page? Go back to the previous one? Type in a new url? For all of those I have to exit fullscreen mode.

I really think this is an edge edge case and would vote for removing that code.

[erikos]

Replying to erikos:

I really think this is an edge edge case and would vote for removing that code.

(The part about handling the escape key in fullscreen mode)

[erikos]

Actually, the fullscreen mode in Browse is strange in the first place because we do not have the options of 'new url', 'reload' or 'back/forward'. At least in ebook mode without shortcuts. All this really sounds strange to me :) Do we have use cases for fullscreen Browse mode? Is that usable as is? Or just a show case?

/me is in question mode today

[manuq]

+1 to erikos, I now think would be better that the first escape press exits fullscreen, and a second one stops loading, if a loading is being processed.

And if changing the toolkit is allowed for this, yes is a better option to do it that way.

[manuq]

This was fixed by Simon and pushed as f9288d2a .

[greenfeld]

We stop as expected but do not reset the busy cursor in Browse-145/OLPC 13.1.0 os9 in two cases:

1. Enter a URL in the search bar and press Enter. Quickly press ESC to abort the load. Press the fullscreen button. You will now see the title of the page (if loaded) with a spinning busy cursor and gray main area (or partial page load). The busy cursor will continue spinning after you exit fullscreen mode.
2. While in fullscreen mode and loading a page quickly press ESC twice to exit fullscreen mode and then abort loading the page. Sometimes the busy cursor keeps spinning in this case as well.

[humitos]

Test case for Esc and spinning cursor

[humitos]

Replying to greenfeld:

We stop as expected but do not reset the busy cursor in Browse-145/OLPC 13.1.0 os9 in two cases:

I found the issue here and it has to do with the load-status property of WebKit.WebView.

The problem is that after a WEBKIT_LOAD_FAILED state it goes to WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT and that state is one of the ones that are of our interest to show the spinning cursor.

I'm not sure if that is a bug on WebKitGTK+ or not. I'm asking on #webkitgtk+

I attached a test case that works with my internet connection speed (it's difficult to catch the exact moment to stop the loading)

[humitos]

I filled a new ticket on the WebKitGTK+ bugtracker to ask about this behaviour.

• ​https://bugs.webkit.org/show_bug.cgi?id=101745

[dnarvaez]

Milestone 0.98 deleted