- Dillo-dev - lists.mailman3.com

PATCH: select new tabs

by onepoint＠starurchin.org

On Mon, Sep 22, 2008 at 12:06:27AM +0000, corvid wrote: > Jeremy wrote: > > > This patch is more of a call for discussion than a fix as it > > fiddles with the UI. It makes Dillo select the new tab after > > creating it. As a special case, middle-clicking a link does not > > select the new tab unless the Shift key is down. (This is the > > same behaviour as Firefox which saves learning a new set of > > reflexes.) What do people think? > > It does seem that "I want to see this in a new tab/window now" has > been more common for me than "I want to look at this in a different > tab/window later". So do you think that a middle button click on a link should always select the new tab? I often go down a list opening up new tabs to look at later so I get frustrated when I have to switch back each time. On the other hand when I create a new blank tab I always want to switch to it immediately so I prefer Dillo to do that for me. Regards, Jeremy Henty

Sept. 22, 2008

Tabs!!!

by corvid＠lavabit.com

I wrote: > Johannes wrote: > > On Fri, Sep 19, 2008 at 03:21:58PM +0000, corvid wrote: > > > I wrote: > > > > Sometimes the window closes when I click on a page. > > > > A quick fprintf() says PUSH causing a recursive FOCUS_CHANGE, > > > > and then HIDE. > > > > (I only had one tab, in case that matters.) > > > > > > Okay, that was not accurate. > > > It looks like PUSH and FOCUS_CHANGE is from clicking > > > and HIDE is from pressing Space to page down. > > > > I'm also seeing that. No idea yet... > > I finally sat down and worked out what the problem is. > It's not only click-space, but that's the most common case. > > <title>...&[character]...</title> > > causes click-[character] to close it. > > So fltk must think that's a shortcut in > the tab text, and it tries to do whatever > it does with shortcuts -- but there's a > shortcut-handling piece missing, so it > just closes the window. It doesn't care about the tab text. It's the UI->window() label stuff that's causing the problem. It appears that you can escape &-as-shortcut in general by doubling them, but I end up with doubled && in the title bar. Why wasn't this a problem in the past? I don't know; maybe it _was_ regarded as a shortcut but handle() took care of it.

Sept. 22, 2008

PATCH: select new tabs

by corvid＠lavabit.com

Jeremy wrote: > This patch is more of a call for discussion than a fix as it fiddles > with the UI. It makes Dillo select the new tab after creating it. As > a special case, middle-clicking a link does not select the new tab > unless the Shift key is down. (This is the same behaviour as Firefox > which saves learning a new set of reflexes.) What do people think? It does seem that "I want to see this in a new tab/window now" has been more common for me than "I want to look at this in a different tab/window later".

Sept. 22, 2008

Tabs!!!

by corvid＠lavabit.com

Johannes wrote: > On Fri, Sep 19, 2008 at 03:21:58PM +0000, corvid wrote: > > I wrote: > > > Sometimes the window closes when I click on a page. > > > A quick fprintf() says PUSH causing a recursive FOCUS_CHANGE, > > > and then HIDE. > > > (I only had one tab, in case that matters.) > > > > Okay, that was not accurate. > > It looks like PUSH and FOCUS_CHANGE is from clicking > > and HIDE is from pressing Space to page down. > > I'm also seeing that. No idea yet... I finally sat down and worked out what the problem is. It's not only click-space, but that's the most common case. <title>...&[character]...</title> causes click-[character] to close it. So fltk must think that's a shortcut in the tab text, and it tries to do whatever it does with shortcuts -- but there's a shortcut-handling piece missing, so it just closes the window.

Sept. 22, 2008

PATCH: select new tabs

by onepoint＠starurchin.org

This patch is more of a call for discussion than a fix as it fiddles with the UI. It makes Dillo select the new tab after creating it. As a special case, middle-clicking a link does not select the new tab unless the Shift key is down. (This is the same behaviour as Firefox which saves learning a new set of reflexes.) What do people think? Regards, Jeremy Henty

Sept. 22, 2008

patch: fonts, utf-8 tabs

by corvid＠lavabit.com

The ui.cc changes are so that I can make a tabs screenshot with Asian script at some point. Similarly, the dialog.cc change is so that I can see Asian characters in source. Kept forgetting to deal with that...

Sept. 21, 2008

patch: making some concessions to aesthetics...

by corvid＠lavabit.com

I've decided to admit that highlighting checkboxes and radio buttons is always hideous. I had been trying to avoid the special case, but I know the horribleness would jump right out at someone trying dillo2... (And I decreased the RELIEF_X_THICKNESS because 5 makes <button>s look weird at my low resolution)

Sept. 21, 2008

cut off word ends with italic fonts

by jcid＠dillo.org

On Fri, Sep 19, 2008 at 04:09:44PM +0200, Johannes Hofmann wrote: > Hi, > > somtimes the end of italic words get cut off e.g. in > > <html> > <body> > <a href="foo">FUU</a>BAR > </body> > </html> > > when seen with vw_fontname="helvetica". > > This is due to a change in html.cc (I think it was introduced with > rev 1.36). This change sets the backgroundColor for all words which > means that for each word, the bounding box get's drawn before the > actual text. This obviously has a performance impact too. > > I'm not sure how to get back to the original behaviour > (backgroundColor == NULL for normal words) without breaking anything. > Can someone please have a look? I can't reproduce the problem here... -- Cheers Jorge.-

Sept. 21, 2008

[patch] minor optimization in StyleAttrs::equals()

by jcid＠dillo.org

On Fri, Sep 19, 2008 at 03:47:15PM +0200, Johannes Hofmann wrote: > Hi, > > this patch adds a missing optimization to StyleAttrs::equals(). > Similar checks are already in place in other equals() methods. Committed. -- Cheers Jorge.-

Sept. 20, 2008

Tabs!!!

by Johannes.Hofmann＠gmx.de

On Fri, Sep 19, 2008 at 03:21:58PM +0000, corvid wrote: > I wrote: > > Sometimes the window closes when I click on a page. > > A quick fprintf() says PUSH causing a recursive FOCUS_CHANGE, > > and then HIDE. > > (I only had one tab, in case that matters.) > > Okay, that was not accurate. > It looks like PUSH and FOCUS_CHANGE is from clicking > and HIDE is from pressing Space to page down. I'm also seeing that. No idea yet... Johannes

Sept. 19, 2008

cut off word ends with italic fonts

by Johannes.Hofmann＠gmx.de

On Fri, Sep 19, 2008 at 03:05:17PM +0000, corvid wrote: > Johannes wrote: > > somtimes the end of italic words get cut off e.g. in > > > > <html> > > <body> > > <a href="foo">FUU</a>BAR > > </body> > > </html> > > > > when seen with vw_fontname="helvetica". > > > > This is due to a change in html.cc (I think it was introduced with > > rev 1.36). This change sets the backgroundColor for all words which > > means that for each word, the bounding box get's drawn before the > > actual text. This obviously has a performance impact too. > > > > I'm not sure how to get back to the original behaviour > > (backgroundColor == NULL for normal words) without breaking anything. > > Can someone please have a look? > > I've had the idea lately (not yet explored) that maybe I could rip out all > of my backgroundColor setting and get the background color > for the fltk widgets if it should turn out that getEmbed()->getBgColor() > has what I want. I'll dig into it. > > PS Is this the same reason that the tops of bold 5's are thin and > the right edge of fixed-width %'s are cut off (using DejaVu fonts, at least)? Not sure. But you can easily check by completely disabling word backgrounds as in attached patch. Cheers, Johannes

Sept. 19, 2008

Tabs!!!

by corvid＠lavabit.com

I wrote: > Sometimes the window closes when I click on a page. > A quick fprintf() says PUSH causing a recursive FOCUS_CHANGE, > and then HIDE. > (I only had one tab, in case that matters.) Okay, that was not accurate. It looks like PUSH and FOCUS_CHANGE is from clicking and HIDE is from pressing Space to page down. Since they're very close to each other on my keyboard and I am so accustomed to the click-and-space sequence, it can come from muscle memory even when I'm paying enough attention to think I know what happened before dillo disappeared.

Sept. 19, 2008

cut off word ends with italic fonts

by corvid＠lavabit.com

Johannes wrote: > somtimes the end of italic words get cut off e.g. in > > <html> > <body> > <a href="foo">FUU</a>BAR > </body> > </html> > > when seen with vw_fontname="helvetica". > > This is due to a change in html.cc (I think it was introduced with > rev 1.36). This change sets the backgroundColor for all words which > means that for each word, the bounding box get's drawn before the > actual text. This obviously has a performance impact too. > > I'm not sure how to get back to the original behaviour > (backgroundColor == NULL for normal words) without breaking anything. > Can someone please have a look? I've had the idea lately (not yet explored) that maybe I could rip out all of my backgroundColor setting and get the background color for the fltk widgets if it should turn out that getEmbed()->getBgColor() has what I want. I'll dig into it. PS Is this the same reason that the tops of bold 5's are thin and the right edge of fixed-width %'s are cut off (using DejaVu fonts, at least)?

Sept. 19, 2008

cut off word ends with italic fonts

by Johannes.Hofmann＠gmx.de

Hi, somtimes the end of italic words get cut off e.g. in <html> <body> <a href="foo">FUU</a>BAR </body> </html> when seen with vw_fontname="helvetica". This is due to a change in html.cc (I think it was introduced with rev 1.36). This change sets the backgroundColor for all words which means that for each word, the bounding box get's drawn before the actual text. This obviously has a performance impact too. I'm not sure how to get back to the original behaviour (backgroundColor == NULL for normal words) without breaking anything. Can someone please have a look? Cheers, Johannes

Sept. 19, 2008

[patch] minor optimization in StyleAttrs::equals()

by Johannes.Hofmann＠gmx.de

Hi, this patch adds a missing optimization to StyleAttrs::equals(). Similar checks are already in place in other equals() methods. Cheers, Johannes

Sept. 19, 2008

[patch] fixup resizable of after tab removal

by Johannes.Hofmann＠gmx.de

Hi, I was seeing crashes when removing tabs. The reason is that when we accidentally remove and delete the resizable of the TabGroup, fltk still tries to access it. Attached patch fixes that. It also moves the hide/showLabel functionality into CustTabGroup. Cheers, Johannes

Sept. 19, 2008

Tabs!!!

by Johannes.Hofmann＠gmx.de

On Thu, Sep 18, 2008 at 11:22:24PM -0400, Jorge Arellano Cid wrote: > On Thu, Sep 18, 2008 at 05:39:43PM +0200, Johannes Hofmann wrote: > > Hi Jorge, > > > > On Wed, Sep 17, 2008 at 06:14:58PM -0400, Jorge Arellano Cid wrote: > > > Hi there, > > > > > > The tabs patch has grown to a quite usable state, so it was > > > committed for you to test. I know we're close to the release so > > > we can still rollback. It's just that I'd much like to have it in > > > the main tree before dillo2 goes out. > > > > > > The patch comes with several assorted cleanups and > > > normalizations. For instance: derived widgets now use the "Cust" > > > prefix instead of "New", the window->user_data() holds a pointer > > > to the current UI, and child widgets get it from there, etc. > > > > > > OK, so to the tabs... > > > > > > This implementation uses one UI per tab, and it's quite fast. > > > You can switch tabs with the mouse, Shift+{Left, Right} or > > > {Ctrl,Ctrl+Shift}+TAB. Use Ctrl+N, or middle click over a link to > > > open a new tab (just as it was with new windows). Ctrl+Q closes > > > the tab. > > > > > > The idea is that in the future the user can dillo2rc-choose > > > whether to use windows or tabs, and windows with tabs. > > > > Very nice! > > The whole tabs or the future dillo2rc idea? ;-) > The whole tabs. I'm not sure I understand the dillo2rc idea correctly (see below). > > > > > > > Some issues: > > > > > > 1.- Tab Tooltips are commented out because FLTK has problems > > > with them. > > > 2.- Shift+{Left, Right} work even when the location is focused. > > > In the latter case you may need to click the right key > > > twice for it to focus and work (don't know why, the Left > > > key works OK). > > > 3.- Currently there's no hidden tabs widget, when there's only > > > one tab (haven't even tried, it may be as simple as > > > hiding the group). > > > > Attached patch implements that. I don't like the global DilloTabs > > variable in uicmd.cc, but the static one > > a_UIcmd_browser_window_new() was not better. Perhaps DilloTabs > > should become member of some new class? > > Committed. > > The good thing of it being a simple variable is that no-tabs windows > is straightforward to implement. I'd say we should support multiple windows with one or more tabs each. In that case DilloTabs can not be a single global variable, as we need one tab widget per window. Cheers, Johannes

Sept. 19, 2008

[patch] switch to standard assert

by Johannes.Hofmann＠gmx.de

On Thu, Sep 18, 2008 at 11:03:32PM -0400, Jorge Arellano Cid wrote: > On Thu, Sep 18, 2008 at 06:29:33PM +0200, Johannes Hofmann wrote: > > On Wed, Sep 17, 2008 at 05:34:12PM -0400, Jorge Arellano Cid wrote: > > > On Wed, Sep 17, 2008 at 05:11:08PM +0200, Johannes Hofmann wrote: > > > > On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > > > > > Johannes wrote: > > > > > > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > > > > > > Johannes wrote: > > > > > > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > > > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > > > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > > > > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > > > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > > > > > > , ...)' can never be inlined because it uses variable argument lists > > > > > > > > > .. > > > > > > > > > > > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > > > > > > (yet) in the code - just const strings. > > > > > > > > So we could change assert to use a const char* argument if I'm not > > > > > > > > missing something. > > > > > > > > > > > > > > For me, grep shows > > > > > > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > > > > > > > > > > > > > Must have missed these... > > > > > > In that case we could get away with something as in attached patch, > > > > > > but we should also consider to just switch to standard assert(). > > > > > > At the moment I would actually prefer the latter. > > > > > > What do others think? > > > > > > > > > > Standard assert() sounds fine to me. > > > > > > > > > > > > > Ok. Here comes the patch. > > > > It also adds a missing check for negative values in SimpleVector > > > > methods. > > > > > > Does this patch show any performance improvement? > > > > > > If it does and has no portability problems, I'm for it too. > > > > > > > I've checked the major BSDs and Linux seems to be fine too. > > According to http://en.wikipedia.org/wiki/C_standard_library it's > > part of the ANSI C library header and was introduced with C99. > > The same holds for stdint.h btw. for which we introduced some macro > > stuff in style.hh. > > I think we should simply rely on C99 compatible headers and include > > those without any conditionals. > > OK, let's try... > > - Assert patch committed. > - Conditional switched to just include stdint.h. > - fail() function removed. Great. I will fix things if this breaks something ;-) Cheers, Johannes

Sept. 19, 2008

Confused by the comment to CustInput::handle()

by jcid＠dillo.org

On Thu, Sep 18, 2008 at 06:01:51AM +0100, Jeremy Henty wrote: > > In dillo2/src/ui.cc, CustInput::handle() has a comment saying that it > disables CTRL+{o,r,HomeKey,EndKey}, yet it only seems to disable > CTRL+o . I think CTRL+r used to be disabled too, so that CTRL+r would > reload the page even if the URL widget had the focus. Here's a > one-line patch to restore that. Yeah, CTRL+r wasn't explicity disabled but it worked. Committed the ignore code at CustInput though (it's cleaner). > > What should we do about CTRL+{HomeKey,EndKey}? I prefer HomeKey and > EndKey to scroll the main window even when the URL has the focus. > Wasn't that the behaviour originally? It certainly doesn't happen > now. AFAIR {HomeKey,EndKey} had always go to begin/end of the text in the Location (when focused), and that's the expected bahaviour. Committed Ctrl+{HomeKey,EndKey} to keep working on the main area even with Location focused. -- Cheers Jorge.-

Sept. 19, 2008

Tabs!!!

by jcid＠dillo.org

On Thu, Sep 18, 2008 at 05:39:43PM +0200, Johannes Hofmann wrote: > Hi Jorge, > > On Wed, Sep 17, 2008 at 06:14:58PM -0400, Jorge Arellano Cid wrote: > > Hi there, > > > > The tabs patch has grown to a quite usable state, so it was > > committed for you to test. I know we're close to the release so > > we can still rollback. It's just that I'd much like to have it in > > the main tree before dillo2 goes out. > > > > The patch comes with several assorted cleanups and > > normalizations. For instance: derived widgets now use the "Cust" > > prefix instead of "New", the window->user_data() holds a pointer > > to the current UI, and child widgets get it from there, etc. > > > > OK, so to the tabs... > > > > This implementation uses one UI per tab, and it's quite fast. > > You can switch tabs with the mouse, Shift+{Left, Right} or > > {Ctrl,Ctrl+Shift}+TAB. Use Ctrl+N, or middle click over a link to > > open a new tab (just as it was with new windows). Ctrl+Q closes > > the tab. > > > > The idea is that in the future the user can dillo2rc-choose > > whether to use windows or tabs, and windows with tabs. > > Very nice! The whole tabs or the future dillo2rc idea? ;-) > > > > Some issues: > > > > 1.- Tab Tooltips are commented out because FLTK has problems > > with them. > > 2.- Shift+{Left, Right} work even when the location is focused. > > In the latter case you may need to click the right key > > twice for it to focus and work (don't know why, the Left > > key works OK). > > 3.- Currently there's no hidden tabs widget, when there's only > > one tab (haven't even tried, it may be as simple as > > hiding the group). > > Attached patch implements that. I don't like the global DilloTabs > variable in uicmd.cc, but the static one > a_UIcmd_browser_window_new() was not better. Perhaps DilloTabs > should become member of some new class? Committed. The good thing of it being a simple variable is that no-tabs windows is straightforward to implement. > > > 4.- There's no close tab icon in the tab label. > > 5.- No warning popup for close window with multiple open tabs. > > > > > > OK, please take it for a ride and test. > > Sometimes Shift-Arrow does not seem to work depending on where the > focus is. Ctrl-Tab seems to work always. Yes, that's what I tried to state in point 2. Well, I have an idea why it happens. The RightKey event gets to a group that handles it as widget navigation and that's why left and right are different. > The window title is not updated when switching between tabs. Is this > intended? Undecided. For tabs, I don't know whether it's a good idea. For single windows certainly it is. This is an open point. -- Cheers Jorge.-

Sept. 19, 2008

[patch] switch to standard assert

by jcid＠dillo.org

On Thu, Sep 18, 2008 at 06:29:33PM +0200, Johannes Hofmann wrote: > On Wed, Sep 17, 2008 at 05:34:12PM -0400, Jorge Arellano Cid wrote: > > On Wed, Sep 17, 2008 at 05:11:08PM +0200, Johannes Hofmann wrote: > > > On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > > > > Johannes wrote: > > > > > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > > > > > Johannes wrote: > > > > > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > > > > > , ...)' can never be inlined because it uses variable argument lists > > > > > > > > .. > > > > > > > > > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > > > > > (yet) in the code - just const strings. > > > > > > > So we could change assert to use a const char* argument if I'm not > > > > > > > missing something. > > > > > > > > > > > > For me, grep shows > > > > > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > > > > > > > > > > Must have missed these... > > > > > In that case we could get away with something as in attached patch, > > > > > but we should also consider to just switch to standard assert(). > > > > > At the moment I would actually prefer the latter. > > > > > What do others think? > > > > > > > > Standard assert() sounds fine to me. > > > > > > > > > > Ok. Here comes the patch. > > > It also adds a missing check for negative values in SimpleVector > > > methods. > > > > Does this patch show any performance improvement? > > > > If it does and has no portability problems, I'm for it too. > > > > I've checked the major BSDs and Linux seems to be fine too. > According to http://en.wikipedia.org/wiki/C_standard_library it's > part of the ANSI C library header and was introduced with C99. > The same holds for stdint.h btw. for which we introduced some macro > stuff in style.hh. > I think we should simply rely on C99 compatible headers and include > those without any conditionals. OK, let's try... - Assert patch committed. - Conditional switched to just include stdint.h. - fail() function removed. -- Cheers Jorge.-

Sept. 19, 2008

Latest libpng fixes crashes

by onepoint＠starurchin.org

I recently looked at why dillo almost always crashed when viewing http://www.packtpub.com/article/transaction-model-of-postgresql and discovered it was happening in the depths of libpng. Luckily libpng-1.2.32 fixes this problem, so if you have been seeing unexpected crashes that might be due to PNG images then I recommend upgrading libpng. Regards, Jeremy Henty

Sept. 18, 2008

[patch] switch to standard assert

by Johannes.Hofmann＠gmx.de

On Wed, Sep 17, 2008 at 05:34:12PM -0400, Jorge Arellano Cid wrote: > On Wed, Sep 17, 2008 at 05:11:08PM +0200, Johannes Hofmann wrote: > > On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > > > Johannes wrote: > > > > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > > > > Johannes wrote: > > > > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > > > > , ...)' can never be inlined because it uses variable argument lists > > > > > > > .. > > > > > > > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > > > > (yet) in the code - just const strings. > > > > > > So we could change assert to use a const char* argument if I'm not > > > > > > missing something. > > > > > > > > > > For me, grep shows > > > > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > > > > > > > Must have missed these... > > > > In that case we could get away with something as in attached patch, > > > > but we should also consider to just switch to standard assert(). > > > > At the moment I would actually prefer the latter. > > > > What do others think? > > > > > > Standard assert() sounds fine to me. > > > > > > > Ok. Here comes the patch. > > It also adds a missing check for negative values in SimpleVector > > methods. > > Does this patch show any performance improvement? > > If it does and has no portability problems, I'm for it too. > I've checked the major BSDs and Linux seems to be fine too. According to http://en.wikipedia.org/wiki/C_standard_library it's part of the ANSI C library header and was introduced with C99. The same holds for stdint.h btw. for which we introduced some macro stuff in style.hh. I think we should simply rely on C99 compatible headers and include those without any conditionals. Cheers, Johannes

Sept. 18, 2008

Tabs!!!

by corvid＠lavabit.com

Sometimes the window closes when I click on a page. A quick fprintf() says PUSH causing a recursive FOCUS_CHANGE, and then HIDE. (I only had one tab, in case that matters.)

Sept. 18, 2008

Tabs!!!

by Johannes.Hofmann＠gmx.de

Hi Jorge, On Wed, Sep 17, 2008 at 06:14:58PM -0400, Jorge Arellano Cid wrote: > Hi there, > > The tabs patch has grown to a quite usable state, so it was > committed for you to test. I know we're close to the release so > we can still rollback. It's just that I'd much like to have it in > the main tree before dillo2 goes out. > > The patch comes with several assorted cleanups and > normalizations. For instance: derived widgets now use the "Cust" > prefix instead of "New", the window->user_data() holds a pointer > to the current UI, and child widgets get it from there, etc. > > OK, so to the tabs... > > This implementation uses one UI per tab, and it's quite fast. > You can switch tabs with the mouse, Shift+{Left, Right} or > {Ctrl,Ctrl+Shift}+TAB. Use Ctrl+N, or middle click over a link to > open a new tab (just as it was with new windows). Ctrl+Q closes > the tab. > > The idea is that in the future the user can dillo2rc-choose > whether to use windows or tabs, and windows with tabs. Very nice! > > Some issues: > > 1.- Tab Tooltips are commented out because FLTK has problems > with them. > 2.- Shift+{Left, Right} work even when the location is focused. > In the latter case you may need to click the right key > twice for it to focus and work (don't know why, the Left > key works OK). > 3.- Currently there's no hidden tabs widget, when there's only > one tab (haven't even tried, it may be as simple as > hiding the group). Attached patch implements that. I don't like the global DilloTabs variable in uicmd.cc, but the static one a_UIcmd_browser_window_new() was not better. Perhaps DilloTabs should become member of some new class? > 4.- There's no close tab icon in the tab label. > 5.- No warning popup for close window with multiple open tabs. > > > OK, please take it for a ride and test. Sometimes Shift-Arrow does not seem to work depending on where the focus is. Ctrl-Tab seems to work always. The window title is not updated when switching between tabs. Is this intended? Cheers, Johannes

Sept. 18, 2008

Confused by the comment to CustInput::handle()

by onepoint＠starurchin.org

On Thu, Sep 18, 2008 at 06:01:51AM +0100, Jeremy Henty wrote: > What should we do about CTRL+{HomeKey,EndKey}? I prefer HomeKey and > EndKey to scroll the main window even when the URL has the focus. Patch attached. Regards, Jeremy Henty

Sept. 18, 2008

Confused by the comment to CustInput::handle()

by onepoint＠starurchin.org

In dillo2/src/ui.cc, CustInput::handle() has a comment saying that it disables CTRL+{o,r,HomeKey,EndKey}, yet it only seems to disable CTRL+o . I think CTRL+r used to be disabled too, so that CTRL+r would reload the page even if the URL widget had the focus. Here's a one-line patch to restore that. What should we do about CTRL+{HomeKey,EndKey}? I prefer HomeKey and EndKey to scroll the main window even when the URL has the focus. Wasn't that the behaviour originally? It certainly doesn't happen now. Regards, Jeremy Henty

Sept. 18, 2008

[patch] switch to standard assert

by corvid＠lavabit.com

Jorge wrote: > On Wed, Sep 17, 2008 at 05:11:08PM +0200, Johannes Hofmann wrote: > > On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > > > Johannes wrote: > > > > but we should also consider to just switch to standard assert(). > > > > At the moment I would actually prefer the latter. > > > > What do others think? > > > > > > Standard assert() sounds fine to me. > > > > > > > Ok. Here comes the patch. > > It also adds a missing check for negative values in SimpleVector > > methods. > > Does this patch show any performance improvement? > > If it does and has no portability problems, I'm for it too. Inlining succeeds now. 8% isn't enough to really notice, but gprof seems consistent with that time being saved. And we should probably rip out fail() as well so that no one ever makes the mistake of using it.

Sept. 18, 2008

Tabs!!!

by jcid＠dillo.org

Hi there, The tabs patch has grown to a quite usable state, so it was committed for you to test. I know we're close to the release so we can still rollback. It's just that I'd much like to have it in the main tree before dillo2 goes out. The patch comes with several assorted cleanups and normalizations. For instance: derived widgets now use the "Cust" prefix instead of "New", the window->user_data() holds a pointer to the current UI, and child widgets get it from there, etc. OK, so to the tabs... This implementation uses one UI per tab, and it's quite fast. You can switch tabs with the mouse, Shift+{Left, Right} or {Ctrl,Ctrl+Shift}+TAB. Use Ctrl+N, or middle click over a link to open a new tab (just as it was with new windows). Ctrl+Q closes the tab. The idea is that in the future the user can dillo2rc-choose whether to use windows or tabs, and windows with tabs. Some issues: 1.- Tab Tooltips are commented out because FLTK has problems with them. 2.- Shift+{Left, Right} work even when the location is focused. In the latter case you may need to click the right key twice for it to focus and work (don't know why, the Left key works OK). 3.- Currently there's no hidden tabs widget, when there's only one tab (haven't even tried, it may be as simple as hiding the group). 4.- There's no close tab icon in the tab label. 5.- No warning popup for close window with multiple open tabs. OK, please take it for a ride and test. Comments, fixes and patches are welcomed. -- Cheers Jorge.-

Sept. 18, 2008

patch: scroll upon resize

by jcid＠dillo.org

On Mon, Sep 15, 2008 at 05:20:01PM +0200, Johannes Hofmann wrote: > Hi, > > On Mon, Sep 15, 2008 at 02:57:27AM +0000, corvid wrote: > > You know how > > - you are scrolled down or to the right > > - you make the viewport larger > > - the scrollbar disappears > > - you can only get the rest of the document back by hitting some > > arrow keys or something > > Yes, I know... > > > > > This seems to fix it, although my usual dw disclaimer applies: > > I don't know whether this is necessarily the right place for > > the change. > > I was thinking about something like this before. > The only problem I see is with tiling window managers like e.g. dwm: > With these windows often get resized, then resized back to their old > size. Currently such a resize cycle does not change the scrolling > position, which is quite nice. > But as most people use ordinary window managers, I'd say we should > commit this fix. Committed. > Perhaps we will find something later that also > keeps the scrolling position after resizing back to the original > size. ACK. > > > > > PS When making the viewport very narrow and then maximizing, I notice > > that the location bar does not go back to showing the whole address. > > I don't know whether that's dillo's fault or fltk's fault. > > > > PPS Amazing how much code we need for dealing with scrolling and scrollbars... > > Yes, that's true. Quite a lot of code actually handles hiding and > showing of the scrollbars and their influence on the size of the viewable > area. > I had proposed a quite radical patch some time ago that moved the > scrollbars from the viewport to the gui (and thereby from dw2 to > dillo2). > This would eliminate all scrollbar related code from dw2. > However as I'm not confident enough that I did not miss any subtle > effects and the current code works fine I would keep things as they > are for the release. > I can update my patch and put it up for discussion after the > release. Yes, it would be good to reconsider it. AFAIR the main reasons it wan't committed were the author wasn't 100% confident, and we were expecting some comments. It can be re-considered after the release. -- Cheers Jorge.-

Sept. 17, 2008

[patch] switch to standard assert

by jcid＠dillo.org

On Wed, Sep 17, 2008 at 05:11:08PM +0200, Johannes Hofmann wrote: > On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > > Johannes wrote: > > > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > > > Johannes wrote: > > > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > > > , ...)' can never be inlined because it uses variable argument lists > > > > > > .. > > > > > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > > > (yet) in the code - just const strings. > > > > > So we could change assert to use a const char* argument if I'm not > > > > > missing something. > > > > > > > > For me, grep shows > > > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > > > > Must have missed these... > > > In that case we could get away with something as in attached patch, > > > but we should also consider to just switch to standard assert(). > > > At the moment I would actually prefer the latter. > > > What do others think? > > > > Standard assert() sounds fine to me. > > > > Ok. Here comes the patch. > It also adds a missing check for negative values in SimpleVector > methods. Does this patch show any performance improvement? If it does and has no portability problems, I'm for it too. -- Cheers Jorge.-

Sept. 17, 2008

[patch] switch to standard assert

by Johannes.Hofmann＠gmx.de

On Tue, Sep 16, 2008 at 09:21:45PM +0000, corvid wrote: > Johannes wrote: > > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > > Johannes wrote: > > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > > , ...)' can never be inlined because it uses variable argument lists > > > > > .. > > > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > > (yet) in the code - just const strings. > > > > So we could change assert to use a const char* argument if I'm not > > > > missing something. > > > > > > For me, grep shows > > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > > > > Must have missed these... > > In that case we could get away with something as in attached patch, > > but we should also consider to just switch to standard assert(). > > At the moment I would actually prefer the latter. > > What do others think? > > Standard assert() sounds fine to me. > Ok. Here comes the patch. It also adds a missing check for negative values in SimpleVector methods. Cheers, Johannes

Sept. 17, 2008

8.68% of my time in assert()?

by corvid＠lavabit.com

Johannes wrote: > On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > > Johannes wrote: > > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > > Turning off bounds checking for get and getRef does get rid of nearly > > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > > , ...)' can never be inlined because it uses variable argument lists > > > > .. > > > > > > Ouch. A short grep shows that variable argument lists are not used > > > (yet) in the code - just const strings. > > > So we could change assert to use a const char* argument if I'm not > > > missing something. > > > > For me, grep shows > > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > > > > Must have missed these... > In that case we could get away with something as in attached patch, > but we should also consider to just switch to standard assert(). > At the moment I would actually prefer the latter. > What do others think? Standard assert() sounds fine to me.

Sept. 16, 2008

8.68% of my time in assert()?

by Johannes.Hofmann＠gmx.de

On Tue, Sep 16, 2008 at 07:20:31PM +0000, corvid wrote: > Johannes wrote: > > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > > Turning off bounds checking for get and getRef does get rid of nearly > > > all of the assert()s, but I was just trying -Winline and discovered: > > > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > > , ...)' can never be inlined because it uses variable argument lists > > > .. > > > > Ouch. A short grep shows that variable argument lists are not used > > (yet) in the code - just const strings. > > So we could change assert to use a const char* argument if I'm not > > missing something. > > For me, grep shows > layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", > Must have missed these... In that case we could get away with something as in attached patch, but we should also consider to just switch to standard assert(). At the moment I would actually prefer the latter. What do others think? Cheers, Johannes PS: I don't see any speed difference with attached patch when loading the huge mysql manual page.

Sept. 16, 2008

8.68% of my time in assert()?

by corvid＠lavabit.com

Johannes wrote: > On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > > Turning off bounds checking for get and getRef does get rid of nearly > > all of the assert()s, but I was just trying -Winline and discovered: > > > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > > , ...)' can never be inlined because it uses variable argument lists > > .. > > Ouch. A short grep shows that variable argument lists are not used > (yet) in the code - just const strings. > So we could change assert to use a const char* argument if I'm not > missing something. For me, grep shows layout.cc: misc::assert (anchor, "anchor '%s' not defined", name); misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d", misc.hh: assert (i < this->num, "Array index out of bounds: %d >= %d",

Sept. 16, 2008

8.68% of my time in assert()?

by Johannes.Hofmann＠gmx.de

On Tue, Sep 16, 2008 at 05:41:04PM +0000, corvid wrote: > Turning off bounds checking for get and getRef does get rid of nearly > all of the assert()s, but I was just trying -Winline and discovered: > > ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': > ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* > , ...)' can never be inlined because it uses variable argument lists > .. Ouch. A short grep shows that variable argument lists are not used (yet) in the code - just const strings. So we could change assert to use a const char* argument if I'm not missing something. > > Earlier I'd said assert() was not cheap, but I was still uncertain about > how -g and -pg might be affecting inlining. Now I _know_ that assert() is > not cheap. > Nice catch! Johannes

Sept. 16, 2008

8.68% of my time in assert()?

by corvid＠lavabit.com

Turning off bounds checking for get and getRef does get rid of nearly all of the assert()s, but I was just trying -Winline and discovered: ./lout/misc.hh: In function 'void lout::misc::assert(bool, const char*, ...)': ./lout/misc.hh:36: warning: function 'void lout::misc::assert(bool, const char* , ...)' can never be inlined because it uses variable argument lists .. Earlier I'd said assert() was not cheap, but I was still uncertain about how -g and -pg might be affecting inlining. Now I _know_ that assert() is not cheap.

Sept. 16, 2008

load_images and multiple windows

by Johannes.Hofmann＠gmx.de

On Tue, Sep 16, 2008 at 12:10:15AM +0000, corvid wrote: > Jorge wrote: > > On Sat, Sep 06, 2008 at 09:00:33PM +0200, Johannes Hofmann wrote: > > > Should load_images be a per window variable, or should we update the > > > buttons in all other windows? > > > > A per window setting looks OK to me. That way people that use > > it only on certain pages (like me) are happy, and those that prefer > > text mode most of the time can have load_images=NO in dillo2rc. > > I just put this together. I won't make much use of it, so please test. > Nice! It even simplifies things a bit. Thanks, Johannes

Sept. 16, 2008

load_images and multiple windows

by corvid＠lavabit.com

Jorge wrote: > On Sat, Sep 06, 2008 at 09:00:33PM +0200, Johannes Hofmann wrote: > > Should load_images be a per window variable, or should we update the > > buttons in all other windows? > > A per window setting looks OK to me. That way people that use > it only on certain pages (like me) are happy, and those that prefer > text mode most of the time can have load_images=NO in dillo2rc. I just put this together. I won't make much use of it, so please test.

Sept. 16, 2008

patch: scroll upon resize

by Johannes.Hofmann＠gmx.de

On Mon, Sep 15, 2008 at 06:54:02PM +0000, corvid wrote: > Johannes wrote: > > Yes, that's true. Quite a lot of code actually handles hiding and > > showing of the scrollbars and their influence on the size of the viewable > > area. > > I had proposed a quite radical patch some time ago that moved the > > scrollbars from the viewport to the gui (and thereby from dw2 to > > dillo2). > > This would eliminate all scrollbar related code from dw2. > > However as I'm not confident enough that I did not miss any subtle > > effects and the current code works fine I would keep things as they > > are for the release. > > I can update my patch and put it up for discussion after the > > release. > > It would at least be interesting to see how things are simplified. > > (Is ScrollGroup involved? I've wondered about it ever since I first > noticed it in fltk) No, it isn't. Scrolling is still be done separately. However, all the nasty stuff is hidden in fltk::scrollrect() already. The patch just makes the scrollbars part of the UI. Cheers, Johannes

Sept. 15, 2008

patch: scroll upon resize

by corvid＠lavabit.com

Johannes wrote: > Yes, that's true. Quite a lot of code actually handles hiding and > showing of the scrollbars and their influence on the size of the viewable > area. > I had proposed a quite radical patch some time ago that moved the > scrollbars from the viewport to the gui (and thereby from dw2 to > dillo2). > This would eliminate all scrollbar related code from dw2. > However as I'm not confident enough that I did not miss any subtle > effects and the current code works fine I would keep things as they > are for the release. > I can update my patch and put it up for discussion after the > release. It would at least be interesting to see how things are simplified. (Is ScrollGroup involved? I've wondered about it ever since I first noticed it in fltk)

Sept. 15, 2008

patch: scroll upon resize

by Johannes.Hofmann＠gmx.de

Hi, On Mon, Sep 15, 2008 at 02:57:27AM +0000, corvid wrote: > You know how > - you are scrolled down or to the right > - you make the viewport larger > - the scrollbar disappears > - you can only get the rest of the document back by hitting some > arrow keys or something Yes, I know... > > This seems to fix it, although my usual dw disclaimer applies: > I don't know whether this is necessarily the right place for > the change. I was thinking about something like this before. The only problem I see is with tiling window managers like e.g. dwm: With these windows often get resized, then resized back to their old size. Currently such a resize cycle does not change the scrolling position, which is quite nice. But as most people use ordinary window managers, I'd say we should commit this fix. Perhaps we will find something later that also keeps the scrolling position after resizing back to the original size. > > PS When making the viewport very narrow and then maximizing, I notice > that the location bar does not go back to showing the whole address. > I don't know whether that's dillo's fault or fltk's fault. > > PPS Amazing how much code we need for dealing with scrolling and scrollbars... Yes, that's true. Quite a lot of code actually handles hiding and showing of the scrollbars and their influence on the size of the viewable area. I had proposed a quite radical patch some time ago that moved the scrollbars from the viewport to the gui (and thereby from dw2 to dillo2). This would eliminate all scrollbar related code from dw2. However as I'm not confident enough that I did not miss any subtle effects and the current code works fine I would keep things as they are for the release. I can update my patch and put it up for discussion after the release. Cheers, Johannes

Sept. 15, 2008

[patch] findbar appearance

by Johannes.Hofmann＠gmx.de

Hi, attached path adjusts the findbar to match the appearance of the panel. Cheers, Johannes

Sept. 15, 2008

patch: scroll upon resize

by corvid＠lavabit.com

You know how - you are scrolled down or to the right - you make the viewport larger - the scrollbar disappears - you can only get the rest of the document back by hitting some arrow keys or something This seems to fix it, although my usual dw disclaimer applies: I don't know whether this is necessarily the right place for the change. PS When making the viewport very narrow and then maximizing, I notice that the location bar does not go back to showing the whole address. I don't know whether that's dillo's fault or fltk's fault. PPS Amazing how much code we need for dealing with scrolling and scrollbars...

Sept. 15, 2008

patch: small mem leaks

by corvid＠lavabit.com

Jeremy kindly provided the memory checking output that went with the valgrind logs from the other day. (I knew about the FullScreen one, but might as well comment it out until someone hooks it up.)

Sept. 13, 2008

More Cache memory corruption

by jcid＠dillo.org

On Thu, Sep 11, 2008 at 10:31:37PM +0000, corvid wrote: > Jeremy wrote: > > Now the bad news: this HTML almost invariably crashes Dillo: > > > > <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" > > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> > > <html xmlns="http://www.w3.org/1999/xhtml"> > > <head> > > <meta > > http-equiv="Content-Type" > > content="application/xhtml+xml; charset=iso-8859-1" > > /> > > </head> > > <body> > > </body> > > </html> > > > > The culprit is the <meta...> tag. Dillo always crashes in one of two > > ways: segmentation fault or glibc detecting memory corruption and > > aborting. In each case the backtrace from gdb is the same (verified > > by running Dillo 100 times (it only failed to crash twice)). The > > backtraces show that the Cache_* functions are freeing things they > > should not. Valgrind confirms this. I'm attaching backtraces and a > > valgrind log. > > That's not bad news. Getting to fix a bug is a good thing. > Try the attached patch. > > Meta causes a repush, which eventually finds its way to > Cache_delayed_process_queue_callback(). In Cache_process_queue(), the fact > that the content type is not viewable causes an abort and the cache entry > is removed. This is bad for my subsequent Cache_unref_data(entry). > (The entries currently have reference counting so that I know when I can > get rid of the UTF8-ized data.) > > > If you put some text above the meta tag, you still have some text visible > that you can right-click to pop up the page menu. Trying the menu options > doesn't crash anything, but...hmmm...are there are implications to not > having a cache entry backing the page? Hmm, that needs to be investigated... I'll hold the patch a bit just in case you find something. -- Cheers Jorge.-

Sept. 12, 2008

[PATCH] Missing include directive for OpenBSD?

by jcid＠dillo.org

On Fri, Sep 12, 2008 at 08:39:10AM +0200, Christian Kellermann wrote: > * Jorge Arellano Cid <jcid(a)dillo.org> [080908 17:18]: > > > > Yes, in dillo2's tree src/colors.h has a check that OpenBSD 4.3 > > is happy with. Please test it. > > Ok, so I duplicated this check to the dw2 tree and rebuild. I can > confirm that this works for OpenBSD 4.3. Committed. -- Cheers Jorge.-

Sept. 12, 2008

More Cache memory corruption

by onepoint＠starurchin.org

On Thu, Sep 11, 2008 at 10:31:37PM +0000, corvid wrote: > Try the attached patch. That fixes it. Thanks! I'm afraid I don't have any answers to your questions: I'm not familiar enough with the cache code. Regards, Jeremy Henty

Sept. 12, 2008

[PATCH] Missing include directive for OpenBSD?

by Christian.Kellermann＠nefkom.net

* Jorge Arellano Cid <jcid(a)dillo.org> [080908 17:18]: > > Yes, in dillo2's tree src/colors.h has a check that OpenBSD 4.3 > is happy with. Please test it. Ok, so I duplicated this check to the dw2 tree and rebuild. I can confirm that this works for OpenBSD 4.3. The file now looks like this (older patches removed): $ cvs diff -bup dw/style.hh Index: dw/style.hh =================================================================== RCS file: /sfhome/cvs/dillo/dw2/dw/style.hh,v retrieving revision 1.5 diff -b -u -p -r1.5 style.hh --- dw/style.hh 18 Jun 2008 13:19:24 -0000 1.5 +++ dw/style.hh 12 Sep 2008 06:39:38 -0000 @@ -1,6 +1,14 @@ #ifndef __DW_STYLE_HH__ #define __DW_STYLE_HH__ +#ifdef HAVE_INTTYPES_H +#include <inttypes.h> +#else +#ifdef HAVE_STDINT_H +#include <stdint.h> +#endif +#endif + #ifndef __INCLUDED_FROM_DW_CORE_HH__ # error Do not include this file directly, use "core.hh" instead. #endif Kind regards, Christian -- You may use my gpg key for replies: pub 1024D/47F79788 2005/02/02 Christian Kellermann (C-Keen)

Sept. 12, 2008

More Cache memory corruption

by corvid＠lavabit.com

Jeremy wrote: > Now the bad news: this HTML almost invariably crashes Dillo: > > <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> > <html xmlns="http://www.w3.org/1999/xhtml"> > <head> > <meta > http-equiv="Content-Type" > content="application/xhtml+xml; charset=iso-8859-1" > /> > </head> > <body> > </body> > </html> > > The culprit is the <meta...> tag. Dillo always crashes in one of two > ways: segmentation fault or glibc detecting memory corruption and > aborting. In each case the backtrace from gdb is the same (verified > by running Dillo 100 times (it only failed to crash twice)). The > backtraces show that the Cache_* functions are freeing things they > should not. Valgrind confirms this. I'm attaching backtraces and a > valgrind log. That's not bad news. Getting to fix a bug is a good thing. Try the attached patch. Meta causes a repush, which eventually finds its way to Cache_delayed_process_queue_callback(). In Cache_process_queue(), the fact that the content type is not viewable causes an abort and the cache entry is removed. This is bad for my subsequent Cache_unref_data(entry). (The entries currently have reference counting so that I know when I can get rid of the UTF8-ized data.) If you put some text above the meta tag, you still have some text visible that you can right-click to pop up the page menu. Trying the menu options doesn't crash anything, but...hmmm...are there are implications to not having a cache entry backing the page? (BTW, I have begun to wonder whether UTF-8 text/* decoding and dicache/imgbuf image/* decoding should develop similarities. Haven't thought about it in any detail, though.)

Sept. 12, 2008