Thread: Re: [Vimprobable-users] [PATCH] Refactured hinting (Page 2)
Vimprobable is a lean web browser optimised for full keyboard control
Brought to you by:
hanness
From: Daniel C. <dan...@gm...> - 2011-08-25 20:24:53
|
Hi! I found a little bug. If I use hinting on a page with only one link, the link is immediately opened like expected, but keeps in hintMode (. or , appear in input box). In the main.c file in input() function following lines seems to cause the behavior by inserting the last input content (arg->s) into the inputbox: /* to avoid things like :open URL :open URL2 or :open :open URL */ gtk_entry_set_text(GTK_ENTRY(inputbox), ""); gtk_editable_insert_text(GTK_EDITABLE(inputbox), arg->s, -1, &pos); if (arg->i & InsertCurrentURL && (url = webkit_web_view_get_uri(webview))) gtk_editable_insert_text(GTK_EDITABLE(inputbox), url, -1, &pos); gtk_widget_grab_focus(inputbox); gtk_editable_set_position(GTK_EDITABLE(inputbox), -1); How can we avoid that vimprobable keeps in hintmode even if the only hint was successfully fired and the mode where set to NormalMode? A possibility to avoid this behavior is to not fire a single hint, but this makes browsing a little slower. Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-25 22:20:40
|
Hi! On Thu, Aug 25, 2011 at 10:24:24PM +0200, Daniel Carl wrote: > Hi! > > I found a little bug. If I use hinting on a page with only one link, the link > is immediately opened like expected, but keeps in hintMode (. or , appear in > input box). > > In the main.c file in input() function following lines seems to cause the > behavior by inserting the last input content (arg->s) into the inputbox: > > /* to avoid things like :open URL :open URL2 or :open :open URL */ > gtk_entry_set_text(GTK_ENTRY(inputbox), ""); > gtk_editable_insert_text(GTK_EDITABLE(inputbox), arg->s, -1, &pos); > if (arg->i & InsertCurrentURL && (url = webkit_web_view_get_uri(webview))) > gtk_editable_insert_text(GTK_EDITABLE(inputbox), url, -1, &pos); > gtk_widget_grab_focus(inputbox); > gtk_editable_set_position(GTK_EDITABLE(inputbox), -1); > > How can we avoid that vimprobable keeps in hintmode even if the only hint was > successfully fired and the mode where set to NormalMode? I found a solution, but I'm not sure if it doesn't insert another bug. To move mentioned lines before the hint script is executed solves the bug. This solves also the bug, that hints aren't create if url was opened and is still present in inputbox (:open url). If 'f' or 'F' is pressed when the url is still in inputbox, the hints where not created and I have to press ESC and start hinting again. Both bugs appear also in current HEAD revision of vimprobable2. Attached patch was done on top of the hinting-refacture. Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-24 15:48:20
Attachments:
screen.png
|
Hi Hannes! On Wed, Aug 24, 2011 at 05:00:16PM +0200, Hannes Schüller wrote: > I've been having one minor issue: On some sites, the hints appear > behind the hinted element (making them unusable). This includes sites > which worked before, like for example this one: > http://www.goodolddays.net/forum/forum.php?id=4 (check the "New Topic" > and "New Poll" buttons). Have you applied all patches? For me it seems to work. I will test it later for previous versions too. Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-16 19:51:35
Attachments:
signature.asc
|
Hi! Daniel Carl <dan...@gm...> wrote: > > - Using the patch, I get the following message on the console: "@1: > > Unsafe JavaScript attempt to access frame with URL > > http://... from frame with URL http://... Domains, protocols and > > ports must match." > The loggin of these message seems to be a bug in webkit. The > violation of same origin policy sould throw an exception instead of > logging only a message on console and retriev undefined values. > https://lists.webkit.org/pipermail/webkit-dev/2010-August/thread.html#13880 > http://code.google.com/p/chromium/issues/detail?id=17325 > > I think we have to live with this behaviour for now. I guess it's not much of a problem. Maybe something to document on the website (wiki/ticket) when it's merged. Any idea of how to handle target attributes? As I said, I think the current behaviour of the patch isn't too bad, but it's also not the expected behaviour ;) So any chance of getting that working as well? Which logic would need to be relocated? Hannes |
From: Daniel C. <dan...@gm...> - 2011-08-17 12:05:06
|
Hi Hannes! On Tue, Aug 16, 2011 at 09:51:21PM +0200, Hannes Schüller wrote: > Any idea of how to handle target attributes? As I said, I think the > current behaviour of the patch isn't too bad, but it's also not the > expected behaviour ;) So any chance of getting that working as well? > Which logic would need to be relocated? I think we should depending of pressed key give additional params from C-Layer to the scripts createHints() function. These param would be set in the object and used to do the right thing in the fire() function. We have to differ between opening new window or not and returning the url of hint or open it. Uzbl uses two variables for this 'newwindow' and 'returnuri', don't know if we need both too. Cases: f (open in current window): - 'newwindow' set to false - dispatch click event on hinted element - links within frames should be opened in the current frame - would also fix some sites using tabmenu with links that are observed for cnklick event -- these will cause the site to be reloaded in the current version because the fire() function retrieves the url to the C-Layer - return nothing to C-Layer F (open in new window): - 'newwindow' set to true - dispatch CTRL-Click event (seems not to work) alternate set target='_blank' dispatch click event and remove target attribute - should work like the pentadactyl plugin, that opens the link into a new window Yank URL (not implemented): - return the src or href attribute value to the C-Layer Other Modes (not implemented): - return some other nice information of the hinted element to the C-Layer I think for the more sofisticated hint actions like the ;... in pentadactyl we have to set a mode for the hinting.js so that the script knows what for an information to return on fire(). Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-17 20:43:45
Attachments:
0001-Added-modes-to-hinting.js.patch
|
Hi Hannes, On Tue, Aug 16, 2011 at 09:51:21PM +0200, Hannes Schüller wrote: > Any idea of how to handle target attributes? As I said, I think the > current behaviour of the patch isn't too bad, but it's also not the > expected behaviour ;) So any chance of getting that working as well? > Which logic would need to be relocated? Attached patch is a first step to handle the target (applies in top o previous patches). The opening isn't done via JavaScript and not from C-Layer as before. I'm really confused by the c code. The patch makes the checks for JavaScript returned string useless, they could be removed. The patch causes that vimprobable keeps in hintmode after a hint was fired. Could you fix this behaviour in the c code? The indented behaviour of the JavaScript is to return "0" if the hint ws focussed or opened, all other modes cause the JavaScript to return a string (for example the url to be yanked). Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-17 21:02:01
|
Oh previous patch causes that the hints would not be removed if the ESC kay is pressed. Can someone check what to change in the C Layer? Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-24 16:25:52
Attachments:
signature.asc
|
Daniel Carl <dan...@gm...> wrote: > On Wed, Aug 24, 2011 at 05:00:16PM +0200, Hannes Schüller wrote: > > I've been having one minor issue: On some sites, the hints appear > > behind the hinted element (making them unusable). This includes > > sites which worked before, like for example this one: > > http://www.goodolddays.net/forum/forum.php?id=4 (check the "New > > Topic" and "New Poll" buttons). > > Have you applied all patches? For me it seems to work. I will test it > later for previous versions too. Yes, patches 1 to 32. Hannes |
From: Daniel C. <dan...@gm...> - 2011-08-24 21:40:59
|
Hi! Hope there will be an end of the patches. Removed unneeded retreiving of the textual content of hintet element. Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-27 13:48:08
Attachments:
signature.asc
|
Hi, this seems to be very solid now. Since this is a rather large change, however, I'd say I make a new release now with just this plus a few pure bugfixes, saving the other feature changes already posted for later. That way, this can get a little more testing. Agreed? Hannes |
From: Daniel C. <dan...@gm...> - 2011-08-27 16:19:44
Attachments:
signature.asc
|
Hi Hannes! On Sat, Aug 27, 2011 at 03:47:49PM +0200, Hannes Schüller wrote: > this seems to be very solid now. Since this is a rather large change, > however, I'd say I make a new release now with just this plus a few > pure bugfixes, saving the other feature changes already posted for > later. That way, this can get a little more testing. Agreed? Yes, I think it should be released to be tested. And I belive it could make some things easier to be implemented if it's released. Maybe we could do some refacturing to the c part too. The main.c is horrible long and could be split into pieces, the global varibales could be replaces by structs and some of hte methods could be split into smaller ones. I suppose it's a big issue to make the code more readable and solid to easily fix bugs and implement fetaures. But I know it't wouldn't be easy to not loose the plot by many patches sent over mailinglist that aren't available in an upstream repository. Maybe we should think about dedicated feature/refacture branches that are more up-to-date than the current master. Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-24 20:25:15
Attachments:
hinting.js.gz
|
Hi Hannes! On Wed, Aug 24, 2011 at 06:25:38PM +0200, Hannes Schüller wrote: > > Have you applied all patches? For me it seems to work. I will test it > > later for previous versions too. > > Yes, patches 1 to 32. I've apllyed all patches from mailbox too on commit 1d44c153d91a15b854d6967c906b2f3259ab795a "description sync" of upstream repository. I got a single warning about whitespace error, but all patches apply well. Also the hints looks right on described page http://www.goodolddays.net/forum/forum.php?id=4. I can't find critical structures in the pages html and css. At the moment I have no idea where to look for the overlayed hints. Please can you check if there's a diff between your hinting.js and my attached version? Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-18 11:02:56
|
Hi! I removed to many lines of code, so that the ESC-Key does not remove the hints. Attaches patches sould be appliet on top of all previous patches. Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-24 20:39:15
Attachments:
signature.asc
|
Daniel Carl <dan...@gm...> wrote: > Please can you check if there's a diff between your hinting.js and my > attached version? The files are exactly identical. Hannes |
From: Daniel C. <dan...@gm...> - 2011-08-24 20:50:05
|
Hi Hannes! Can you try to find the reason for this missbehavior, I couldn't reproduce it here. Maybe you can try to change the z-index, remove userdefined configuration or get into the issue by inspecting hints and hintet elements via webinspector. Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-19 21:24:33
|
Hi! I've fixed some minor bugs in the new hinting.js. Daniel |
From: Daniel C. <dan...@gm...> - 2011-08-20 15:04:04
|
Hi! I hope the hinting is now functional like before refacturing. Fixed bug that caused vimprobable to keep in hintmode, even if hint was fired. Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-24 21:13:19
Attachments:
signature.asc
|
Hi! Daniel Carl <dan...@gm...> wrote: > Can you try to find the reason for this missbehavior, I couldn't > reproduce it here. Maybe you can try to change the z-index, remove > userdefined configuration or get into the issue by inspecting hints > and hintet elements via webinspector. I think I've got it pinned down. You set z-index to 10000000. This does not seem to work. In fact, it is stripped out completely: The hints have no z-index attribute at all! Lowering this value to 1000, for example, makes it work again. I'd say the problem is a difference in the Webkit versions. 100000 still works for me, 1000000 doesn't. So I'd say we use something like 100000 for reasons of compatibility. Hannes |
From: Daniel C. <dan...@gm...> - 2011-08-25 06:25:19
|
Hi Hannes! On Wed, Aug 24, 2011 at 11:13:02PM +0200, Hannes Schüller wrote: > I'd say the problem is a difference in the Webkit versions. 100000 > still works for me, 1000000 doesn't. So I'd say we use something like > 100000 for reasons of compatibility. Thank you for inspecting the issue. Can you make the patch with al lower value? I'm wondering that 10000000 doesn't work. In http://www.puidokas.com/max-z-index/ mentioned restriction are higher than our current value. But on most serious pages also a z-index of 100000 should fit our needs. Daniel |
From: Hannes S. <ha...@yl...> - 2011-08-29 16:00:01
Attachments:
signature.asc
|
I believe I may have found another bug (just in time *after* the public release): - Go to a site with *many* links. For demonstration purposes, let's take http://www.imdb.com/title/tt0064904/ - Scroll down so that the "Message Boards" heading is on top of the window. - Activate hinting by pressing "f". - Notice that the message board topics are hinted, but using numbers in the 40s instead of starting with 1. All the links on the bottom of the page ("Explore more") are not hinted at all. A strange hint appears in the upper left corner of the window (47 for me). Hannes |