1:09 a.m.
Hi,
One aspect of the tabbed browsing that I felt needed addressing; it so
happened it was a relatively straightforward patch after tracking down
the necessary details :-)
The 256 length limit is as in browser_window_go_post in /desktop/browser.c
I hope the style is right, I've been registering what you've been saying
today, as well as looking at the way it's presented in the sources
Best
--
Mark
http://www.halloit.com
Key ID 046B65CF
2:05 a.m.
On Sun, 2009-03-29 at 18:09 +0200, netsurf.list.markie1(a)dfgh.net wrote:
One aspect of the tabbed browsing that I felt needed addressing; it
so
happened it was a relatively straightforward patch after tracking down
the necessary details :-)
Thanks for this. Comments below.
The 256 length limit is as in browser_window_go_post in
/desktop/browser.c
I this limit needs to die.
Index: gtk/gtk_scaffolding.c
===================================================================
--- gtk/gtk_scaffolding.c (revision 6935)
+++ gtk/gtk_scaffolding.c (working copy)
@@ -328,6 +328,8 @@
void nsgtk_window_update_back_forward(struct gtk_scaffolding *g)
{
int width, height;
+ char url_buf[256];
+
struct browser_window *bw =
nsgtk_get_browser_for_gui(g->top_level);
gtk_widget_set_sensitive(GTK_WIDGET(g->back_button),
@@ -344,6 +346,19 @@
gtk_widget_set_sensitive(GTK_WIDGET(glade_xml_get_widget(g->popup_xml,
"popupForward")),
history_forward_available(bw->history));
+ /* update the url bar, particularly necessary when tabbing */
+ if (bw->current_content) {
+ browser_window_update(bw);
Can you explain why this is needed? You mentioned something about the
first tab being odd.
+ if (bw->frag_id) {
+ snprintf(url_buf, sizeof url_buf, "%s#%s",
+ bw->current_content->url,
bw->frag_id);
+ } else {
+ snprintf(url_buf, sizeof url_buf, "%s",
+ bw->current_content->url);
+ }
+ url_buf[sizeof url_buf - 1] = 0;
+ gui_window_set_url(bw->window, url_buf);
+ }
Modulo the length limit, this is fine.
Index: gtk/gtk_tabs.c
===================================================================
--- gtk/gtk_tabs.c (revision 6935)
+++ gtk/gtk_tabs.c (working copy)
@@ -107,7 +107,7 @@
gtk_container_add(GTK_CONTAINER(button), close);
gtk_button_set_focus_on_click(GTK_BUTTON(button), FALSE);
gtk_button_set_relief(GTK_BUTTON(button), GTK_RELIEF_NONE);
- gtk_widget_set_tooltip_text(button, "Close this tab.");
+ gtk_widget_set_tooltip_text(button, "Close this tab");
Is there a reason for this change?
J.
2:24 a.m.
Hi John-Mark,
John-Mark Bell - jmb(a)netsurf-browser.org wrote:
> + browser_window_update(bw);
>
Can you explain why this is needed? You mentioned something about the
first tab being odd.
It is, the LHS of the page draws, not the RHS; though that
browser_window_update() call has no effect, even putting it ahead of the
conditional;
> - gtk_widget_set_tooltip_text(button, "Close this tab.");
> + gtk_widget_set_tooltip_text(button, "Close this tab");
>
Is there a reason for this change?
J.
It's not important, it had looked better at the time, though I now see
that most of the tooltips have full stops
here's the patch re-improved, slimmed down
Best
Mark
http://www.halloit.com
Key ID 046B65CF
2:32 a.m.
On Sun, 2009-03-29 at 19:24 +0200, netsurf.list.markie1(a)dfgh.net wrote:
John-Mark Bell - jmb(a)netsurf-browser.org wrote:
> > + browser_window_update(bw);
> >
>
> Can you explain why this is needed? You mentioned something about the
> first tab being odd.
>
It is, the LHS of the page draws, not the RHS; though that
browser_window_update() call has no effect, even putting it ahead of
the conditional;
Ok. That's something else entirely. I believe that Mike Lester has a fix
for that. Mike?
here's the patch re-improved, slimmed down
Thanks. Modulo the fixed-size buffer, that looks fine. I suspect the
solution to the fixed buffer is buf = malloc(strlen(url) +
strlen(frag_id) + <length of separators + \0>);
J.
2:58 a.m.
Hi John-Mark,
The only reason I would have been shy of malloc() – as well as the fact
that the fixed-length is how it was written in browser_window_go_post()
– would be that stack allocation is normally much faster, plus it's, I
suppose, a local variable, so the stack memory taken is quickly
released? Though I'm not sure how that interacts with the const
definition in void gui_window_set_url(struct gui_window *_g, const char
*url); [gtk_scaffolding.c 1418] memory allocation in C not being the
easiest job to get just right :-)
As it's currently a tiny amount of memory, the RISC OS limit you
referred to may be the most important factor, unless you really want it
changed for the sake of tidiness
Best
Mark
http://www.halloit.com
Key ID 046B65CF
John-Mark Bell - jmb(a)ecs.soton.ac.uk wrote:
On Sun, 2009-03-29 at 19:24 +0200, netsurf.list.markie1(a)dfgh.net
wrote:
> John-Mark Bell - jmb(a)netsurf-browser.org wrote:
>
>>> + browser_window_update(bw);
>>>
>>>
>> Can you explain why this is needed? You mentioned something about the
>> first tab being odd.
>>
>>
> It is, the LHS of the page draws, not the RHS; though that
> browser_window_update() call has no effect, even putting it ahead of
> the conditional;
>
Ok. That's something else entirely. I believe that Mike Lester has a fix
for that. Mike?
> here's the patch re-improved, slimmed down
>
Thanks. Modulo the fixed-size buffer, that looks fine. I suspect the
solution to the fixed buffer is buf = malloc(strlen(url) +
strlen(frag_id) + <length of separators + \0>);
J.
3:13 a.m.
On Sun, 2009-03-29 at 19:58 +0200, netsurf.list.markie1(a)dfgh.net wrote:
The only reason I would have been shy of malloc() – as well as the
fact that the fixed-length is how it was written in
browser_window_go_post() – would be that stack allocation is normally
much faster, plus it's, I suppose, a local variable, so the stack
memory taken is quickly released?
Sure. URLs have unbounded length, however, so putting them on the stack
isn't necessarily a good idea.
Though I'm not sure how that interacts with the const definition
in
void gui_window_set_url(struct gui_window *_g, const char *url);
[gtk_scaffolding.c 1418] memory allocation in C not being the easiest
job to get just right :-)
gui_window_set_url must copy the data passed to it.
As it's currently a tiny amount of memory, the RISC OS limit you
referred to may be the most important factor, unless you really want
it changed for the sake of tidiness
Well, the point is that the core code shouldn't be limited by RISC OS
(or any other platform frontend). So, browser_window_go_post needs to
change, regardless.
J.
7:36 p.m.
Hi,
So I've tracked down more applications of the 256-byte Url limit,
putting them all together in a function looks a good idea, that could
then be modified to cope with RISC as appropriate
Best
Mark
http://www.halloit.com
Key ID 046B65CF
John-Mark Bell - jmb(a)netsurf-browser.org wrote:
On Sun, 2009-03-29 at 19:58 +0200, netsurf.list.markie1(a)dfgh.net
wrote:
> The only reason I would have been shy of malloc() – as well as the
> fact that the fixed-length is how it was written in
> browser_window_go_post() – would be that stack allocation is normally
> much faster, plus it's, I suppose, a local variable, so the stack
> memory taken is quickly released?
>
Sure. URLs have unbounded length, however, so putting them on the stack
isn't necessarily a good idea.
> Though I'm not sure how that interacts with the const definition in
> void gui_window_set_url(struct gui_window *_g, const char *url);
> [gtk_scaffolding.c 1418] memory allocation in C not being the easiest
> job to get just right :-)
>
gui_window_set_url must copy the data passed to it.
> As it's currently a tiny amount of memory, the RISC OS limit you
> referred to may be the most important factor, unless you really want
> it changed for the sake of tidiness
>
Well, the point is that the core code shouldn't be limited by RISC OS
(or any other platform frontend). So, browser_window_go_post needs to
change, regardless.
J.
11:01 p.m.
On Mon, 2009-03-30 at 12:36 +0200, netsurf.list.markie1(a)dfgh.net wrote:
So I've tracked down more applications of the 256-byte Url
limit,
putting them all together in a function looks a good idea, that could
then be modified to cope with RISC as appropriate
In general, this looks ok. Please ensure lines are wrapped to 80
columns, using 2 tabs for indenting continuation lines.
Index: desktop/browser.c
===================================================================
--- desktop/browser.c (revision 6935)
+++ desktop/browser.c (working copy)
@@ -1092,6 +1079,27 @@
browser_window_set_scale_internal(&bw->iframes[i],
scale);
}
+void browser_window_refresh_url_bar(struct browser_window *bw, char
*url, char *frag)
+{
url and frag should be const char *.
+ int l = strlen(url);
+ l += (frag) ? strlen(frag) : 0;
+ char *url_buf = (char *)malloc((l + 2) * (sizeof (char)));
url_buf will need to be declared before the l += line, else you'll break
the BeOS build. (Despite what any documentation says, the core must be
C89 these days.)
sizeof(char) is defined to be 1, so you can remove the * (sizeof(char)).
The cast of the result of malloc is unnecessary -- void * is implicitly
cast to the lvalue's type.
+ if (url_buf) {
+ if (frag) {
+ snprintf(url_buf, l, "%s#%s",
+ url, frag);
l + 1, surely?
+ *(url_buf + l + 1) = 0;
'\0' is clearer.
+ } else {
+ snprintf(url_buf, l, "%s",
+ url);
+ *(url_buf + l) = 0;
+ }
+ gui_window_set_url(bw->window, url_buf);
+ free(url_buf);
+ } else {
+ warn_user("NoMemory", 0);
+ }
+}
/**
* Locate a browser window in the specified stack according.
Index: desktop/browser.h
===================================================================
--- desktop/browser.h (revision 6935)
+++ desktop/browser.h (working copy)
@@ -231,6 +231,8 @@
void browser_window_reformat(struct browser_window *bw, int width,
int height);
void browser_window_set_scale(struct browser_window *bw, float scale,
bool all);
+void browser_window_refresh_url_bar(struct browser_window *bw, char *
url, char * frag);
+
Similarly, constify.
J.
5295
days inactive
5296
days old
netsurf-dev@netsurf-browser.org
7 comments
3 participants
participants (3)
-
John-Mark Bell -
John-Mark Bell -
netsurf.list.markie1＠dfgh.net