2009/3/4 Rob Kendrick <rjek(a)netsurf-browser.org&gt; Index: desktop/history_core.c =================================================================== --- desktop/history_core.c (wersja 6688) +++ desktop/history_core.c (kopia robocza) -29,6 +29,7 @@ #include "content/content.h" #include "content/urldb.h" #include "css/css.h" +#include "desktop/browser.h" #include "desktop/gui.h" #include "desktop/history_core.h" #include "desktop/plotters.h" -63,6 +64,8 @@ int x; /**< Position of node. */ int y; /**< Position of node. */ struct bitmap *bitmap; /**< Thumbnail bitmap, or 0. */ + int sx; /**< X scroll offset. */ + int sy; /**< Y scroll offset. */ }; /** History tree for a window. */ -257,6 +260,8 @@ entry->forward = entry->forward_pref = entry->forward_last = 0; entry->children = 0; entry->bitmap = 0; + entry->sx = -1; + entry->sy = -1; if (history->current) { if (history->current->forward_last) history->current->forward_last->next = entry; -455,6 +460,10 @@ else url = entry->page.url; + /* the scroll offsets have to be remembered before changing + history->current */ + history_update_current_scroll(bw); + if (new_window) { current = history->current; history->current = entry; -717,3 +726,43 @@ return 0; } + +/** + * Save in the history the scroll offsets of the current page + * \param bw browser window with the page + */ + +void history_update_current_scroll(struct browser_window *bw) +{ + int sx, sy; + struct history *history = bw->history; + + if (!history || !history->current) + return; + + gui_window_get_scroll(bw->window, &sx, &sy); + history->current->sx = sx; + history->current->sy = sy; + printf("setting scroll\t%i\t%i for %s\n", sx, sy, history->current->page.url); +} + +/** + * Load from the history the scroll offsets of the current page + * \param bw browser window with the page + * \param sx updated to x offset + * \param sy updated to y offset + * \return true on success + */ + +bool history_get_current_scroll(struct browser_window *bw, int *sx, int *sy) +{ + struct history *history = bw->history; + + if (!history || !history->current) + return false; + + *sx = history->current->sx; + *sy = history->current->sy; + printf("getting scroll\t%i\t%i for %s\n", *sx, *sy, history->current->page.url); + return true; +} Index: desktop/history_core.h =================================================================== --- desktop/history_core.h (wersja 6688) +++ desktop/history_core.h (kopia robocza) -44,5 +44,6 @@ bool history_click(struct browser_window *bw, struct history *history, int x, int y, bool new_window); const char *history_position_url(struct history *history, int x, int y); - +void history_update_current_scroll(struct browser_window *bw); +bool history_get_current_scroll(struct browser_window *bw, int *sx, int *sy); #endif Index: desktop/browser.c =================================================================== --- desktop/browser.c (wersja 6688) +++ desktop/browser.c (kopia robocza) -313,7 +313,12 @@ free(url2); return; } - + /* we have to remember the scroll offsets for the current page + if we don't return to a page in the history (otherwise it's done by the + history code)*/ + if (history_add) + history_update_current_scroll(bw); + free(bw->frag_id); bw->frag_id = NULL; -766,7 +771,7 @@ bool scroll_to_top) { struct box *pos; - int x, y; + int x, y, sx, sy; if (!bw->current_content) return; -778,12 +783,17 @@ gui_window_update_extent(bw->window); - if (scroll_to_top) + if (scroll_to_top) { gui_window_set_scroll(bw->window, 0, 0); - - /* todo: don't do this if the user has scrolled */ + sx = -1; + } else if (!history_get_current_scroll(bw, &sx, &sy)) + sx = -1; + + /* if the page was scrolled before return to that position */ + if (sx != -1) { + gui_window_set_scroll(bw->window, sx, sy); /* if frag_id exists, then try to scroll to it */ - if (bw->frag_id && bw->current_content->type == CONTENT_HTML) { + } else if (bw->frag_id && bw->current_content->type == CONTENT_HTML) { if ((pos = box_find_by_id(bw->current_content->data.html.layout, bw->frag_id)) != 0) { box_coords(pos, &x, &y); gui_window_set_scroll(bw->window, x, y);

On Mon, 2009-03-09 at 22:11 +0100, Paweł Blokus wrote: Thanks for this. Your approach is mostly fine. I've commented in detail below. Note for RISC OS, Amiga, BeOS/Haiku, Framebuffer port maintainers: you'll want to ensure the scroll offsets are reported to the core correctly once this patch goes in. I've little to no knowledge of the intricacies of GTK -- this is one for Rob or Mike. (Similarly, below, I've not commented on your GTK frontend changes at all) In comparison to performing the required redraw, setting two integers in a struct somewhere should have no performance impact at all. Could this not be void history_set_current_scroll(struct history *history, int sx, int sy) ? With the API change suggested above, this upcall is redundant, as the frontend will provide the scroll offsets directly. Similarly, s/struct browser_window *bw/struct history *history/ s/struct browser_window *bw/struct history *history/ As above. The clearing here is to ensure the fragment identifier wins, right? Small nit: make this "} else" J.

Now, if one is moving on the same page using frag_id, new entries are added to the history (what's the usual behaviour of other browsers). I also found and removed a bug - while clicking on the local history on entries with the same url the window didn't get refreshed so that the highlight didn't get changed (this might need changing in the other platforms code too). Index: gtk/gtk_window.c =================================================================== --- gtk/gtk_window.c (wersja 6792) +++ gtk/gtk_window.c (kopia robocza) @@ -20,6 +20,7 @@ #include <inttypes.h> #include "gtk/gtk_window.h" #include "desktop/browser.h" +#include "desktop/history_core.h" #include "desktop/options.h" #include "desktop/textinput.h" #include "desktop/selection.h" @@ -52,6 +53,7 @@ gpointer); static gboolean nsgtk_window_size_allocate_event(GtkWidget *, GtkAllocation *, gpointer); +static void nsgtk_window_scrolled(GtkAdjustment *, gpointer); /* Other useful bits */ static void nsgtk_redraw_caret(struct gui_window *g); @@ -85,6 +87,8 @@ { struct gui_window *g; /**< what we're creating to return */ GtkPolicyType scrollpolicy; + GtkAdjustment *vadj; + GtkAdjustment *hadj; g = malloc(sizeof(*g)); if (!g) { @@ -140,6 +144,9 @@ GTK_SHADOW_NONE); g->viewport = GTK_VIEWPORT(gtk_bin_get_child(GTK_BIN(g->scrolledwindow))); g->tab = NULL; + + vadj = gtk_viewport_get_vadjustment(g->viewport); + hadj = gtk_viewport_get_hadjustment(g->viewport); if (bw->parent != NULL) /* Attach ourselves into our parent at the right point */ @@ -225,6 +232,8 @@ nsgtk_window_keypress_event, g); CONNECT(g->viewport, "size_allocate", nsgtk_window_size_allocate_event, g); + CONNECT(vadj, "value-changed", nsgtk_window_scrolled, g); + CONNECT(hadj, "value-changed", nsgtk_window_scrolled, g); return g; } @@ -582,6 +591,16 @@ return TRUE; } +void nsgtk_window_scrolled(GtkAdjustment *ga, gpointer data) +{ + struct gui_window *g = data; + int sx, sy; + + if (!g->setting_scroll) { + gui_window_get_scroll(g->bw->window, &sx, &sy); + history_set_current_scroll(g->bw->history, sx, sy); + } +} void nsgtk_reflow_all_windows(void) { @@ -718,8 +737,10 @@ if (y > (vupper - vpage)) y = vupper - vpage; + g->setting_scroll = true; gtk_adjustment_set_value(vadj, y); gtk_adjustment_set_value(hadj, x); + g->setting_scroll = false; } Index: gtk/gtk_window.h =================================================================== --- gtk/gtk_window.h (wersja 6792) +++ gtk/gtk_window.h (kopia robocza) @@ -34,6 +34,7 @@ int caretx, carety, careth; gui_pointer_shape current_pointer; int last_x, last_y; + bool setting_scroll; /* Within GTK, a gui_window is a scrolled window * with a viewport inside Index: gtk/gtk_scaffolding.c =================================================================== --- gtk/gtk_scaffolding.c (wersja 6792) +++ gtk/gtk_scaffolding.c (kopia robocza) @@ -1064,6 +1064,12 @@ { struct gtk_scaffolding *gw = (struct gtk_scaffolding *)g; + /* if entries of the same url but different frag_ids have been added + * the history needs redrawing (what is done in the throbber code in + * other cases) + */ + nsgtk_window_update_back_forward(gw); + gtk_widget_show(GTK_WIDGET(gw->history_window->window)); gdk_window_raise(GTK_WIDGET(gw->history_window->window)->window); @@ -1149,6 +1155,7 @@ history_click(bw, bw->history, event->x, event->y, false); + nsgtk_window_update_back_forward(bw->window->scaffold); return TRUE; } Index: riscos/window.c =================================================================== --- riscos/window.c (wersja 6792) +++ riscos/window.c (kopia robocza) @@ -2537,7 +2537,7 @@ browser_window_set_scale(g->bw, scale, true); // g->reformat_pending = true; // if ((content) && (content->type != CONTENT_HTML)) -// browser_window_update(g->bw, false); +// browser_window_update(g->bw); // browser_reformat_pending = true; } return true; Index: desktop/history_core.c =================================================================== --- desktop/history_core.c (wersja 6792) +++ desktop/history_core.c (kopia robocza) @@ -29,6 +29,7 @@ #include "content/content.h" #include "content/urldb.h" #include "css/css.h" +#include "desktop/browser.h" #include "desktop/gui.h" #include "desktop/history_core.h" #include "desktop/plotters.h" @@ -63,6 +64,8 @@ int x; /**< Position of node. */ int y; /**< Position of node. */ struct bitmap *bitmap; /**< Thumbnail bitmap, or 0. */ + int sx; /**< X scroll offset. */ + int sy; /**< Y scroll offset. */ }; /** History tree for a window. */ @@ -259,6 +262,8 @@ entry->forward = entry->forward_pref = entry->forward_last = 0; entry->children = 0; entry->bitmap = 0; + entry->sx = -1; + entry->sy = -1; if (history->current) { if (history->current->forward_last) history->current->forward_last->next = entry; @@ -755,3 +760,37 @@ return 0; } + +/** + * Save in the history the scroll offsets of the current page + * \param history history with the page + * \param sx x offset to be set + * \param sy y offset to be set + */ + +void history_set_current_scroll(struct history *history, int sx, int sy) +{ + if (!history || !history->current) + return; + + history->current->sx = sx; + history->current->sy = sy; +} + +/** + * Load from the history the scroll offsets of the current page + * \param history history with the page + * \param sx updated to x offset + * \param sy updated to y offset + * \return true on success + */ + +bool history_get_current_scroll(struct history *history, int *sx, int *sy) +{ + if (!history || !history->current) + return false; + + *sx = history->current->sx; + *sy = history->current->sy; + return true; +} Index: desktop/history_core.h =================================================================== --- desktop/history_core.h (wersja 6792) +++ desktop/history_core.h (kopia robocza) @@ -46,5 +46,7 @@ bool history_click(struct browser_window *bw, struct history *history, int x, int y, bool new_window); const char *history_position_url(struct history *history, int x, int y); +void history_set_current_scroll(struct history *history, int sx, int sy); +bool history_get_current_scroll(struct history *history, int *sx, int *sy); #endif Index: desktop/browser.c =================================================================== --- desktop/browser.c (wersja 6792) +++ desktop/browser.c (kopia robocza) @@ -277,7 +277,7 @@ void browser_window_go_post(struct browser_window *bw, const char *url, char *post_urlenc, struct form_successful_control *post_multipart, - bool history_add, const char *referer, bool download, + bool history_add_arg, const char *referer, bool download, bool verifiable, const char *parent_url) { struct content *c; @@ -347,7 +347,10 @@ if (same_url && !post_urlenc && !post_multipart && !strchr(url2, '?')) { free(url2); - browser_window_update(bw, false); + if (history_add_arg) + history_add(bw->history, bw->current_content, + bw->frag_id); + browser_window_update(bw); snprintf(url_buf, sizeof url_buf, "%s#%s", bw->current_content->url, bw->frag_id); url_buf[sizeof url_buf - 1] = 0; @@ -364,7 +367,7 @@ LOG(("Loading '%s' width %i, height %i", url2, width, height)); browser_window_set_status(bw, messages_get("Loading")); - bw->history_add = history_add; + bw->history_add = history_add_arg; c = fetchcache(url2, browser_window_callback, (intptr_t) bw, 0, width, height, false, post_urlenc, post_multipart, verifiable, download); @@ -452,7 +455,7 @@ snprintf(url, sizeof url, "%s", c->url); url[sizeof url - 1] = 0; gui_window_set_url(bw->window, url); - browser_window_update(bw, true); + browser_window_update(bw); content_open(c, bw, 0, 0, 0, 0); browser_window_set_status(bw, c->status_message); @@ -491,7 +494,7 @@ case CONTENT_MSG_DONE: assert(bw->current_content == c); - browser_window_update(bw, false); + browser_window_update(bw); browser_window_set_status(bw, c->status_message); browser_window_stop_throbber(bw); history_update(bw->history, c); @@ -541,7 +544,7 @@ } if (bw->move_callback) bw->move_callback(bw, bw->caret_p); - browser_window_update(bw, false); + browser_window_update(bw); break; case CONTENT_MSG_REDRAW: @@ -774,11 +777,10 @@ * \param scroll_to_top move view to top of page */ -void browser_window_update(struct browser_window *bw, - bool scroll_to_top) +void browser_window_update(struct browser_window *bw) { struct box *pos; - int x, y; + int x, y, sx, sy; if (!bw->current_content) return; @@ -790,17 +792,20 @@ gui_window_update_extent(bw->window); - if (scroll_to_top) - gui_window_set_scroll(bw->window, 0, 0); - - /* todo: don't do this if the user has scrolled */ + if (!history_get_current_scroll(bw->history, &sx, &sy)) + sx = -1; + + /* if the page was scrolled before return to that position */ + if (sx != -1) { + gui_window_set_scroll(bw->window, sx, sy); /* if frag_id exists, then try to scroll to it */ - if (bw->frag_id && bw->current_content->type == CONTENT_HTML) { + } else if (bw->frag_id && bw->current_content->type == CONTENT_HTML) { if ((pos = box_find_by_id(bw->current_content->data.html.layout, bw->frag_id)) != 0) { box_coords(pos, &x, &y); gui_window_set_scroll(bw->window, x, y); } - } + } else + gui_window_set_scroll(bw->window, 0, 0); gui_window_redraw_window(bw->window); } @@ -1072,7 +1077,7 @@ c = bw->current_content; if (c) { if (!content_can_reformat(c)) { - browser_window_update(bw, false); + browser_window_update(bw); } else { bw->reformat_pending = true; browser_reformat_pending = true; Index: desktop/browser.h =================================================================== --- desktop/browser.h (wersja 6792) +++ desktop/browser.h (kopia robocza) @@ -223,7 +223,7 @@ const char *url, const char *referrer, bool history_add); void browser_window_download(struct browser_window *bw, const char *url, const char *referrer); -void browser_window_update(struct browser_window *bw, bool scroll_to_top); +void browser_window_update(struct browser_window *bw); void browser_window_stop(struct browser_window *bw); void browser_window_reload(struct browser_window *bw, bool all); void browser_window_destroy(struct browser_window *bw); Index: amiga/gui.c =================================================================== --- amiga/gui.c (wersja 6792) +++ amiga/gui.c (kopia robocza) @@ -1353,7 +1353,7 @@ GetAttr(SPACE_AreaBox,gwin->gadgets[GID_BROWSER],(ULONG *)&bbox); p96RectFill(gwin->win->RPort,bbox->Left,bbox->Top,bbox->Width+bbox->Left,bbox->Height+bbox->Top,0xffffffff); - browser_window_update(gwin->bw,false); + browser_window_update(gwin->bw); // if((gwin->bw->window->scrollx) || (gwin->bw->window->scrolly)) // {