October 2008 - Dillo-dev - lists.mailman3.com

[patch]: fix the download GUI resize bug

by jcid＠dillo.org

On Wed, Oct 29, 2008 at 09:16:07PM +0000, Jeremy Henty wrote: > On Wed, Oct 29, 2008 at 01:38:14PM -0300, Jorge Arellano Cid wrote: > > On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote: > > > > > > There's a bug in the downloads GUI that stops the download items > > > resizing when the window does. The resizable of the window should > > > be the scrollgroup, not the packedgroup. Fixing this means we > > > don't need to explicitly resize the scrollgroup after removing an > > > item (because the window has already resized it) so I took that > > > out. Patch attached. > > > > Oh, please excuse me not understanding what the bug is and how to > > reproduce it. Please explain with apples and pears! :-) > > OK, fruit 'n' veg coming up! :-) > > Symptom: > > If you change the width of the download GUI window its contents do not > respond. This is annoying because (a) if you narrow the window to > save space then it truncates its contents at the right-hand side > rather than gracefully resizing them, and (b) if you widen the window > hoping that it will make the display more legible by widening the text > labels etc. then nothing happens. It's an easy bug to miss because > the result is not obviously wrong, just poor and inconvenient. Once > you've seen the correct behaviour the improvement is obvious. I still don't get it... Here, with the unpatched code, the download dpi resizes without problems when narrowing the window (fltk-r6403), and labels are widened too. And more, the unpatched code responds to clicking "done" by removing the corresponding item (when alone and when in a list). The patched code, here, sometimes doesn't remove the widget when clicking "done", and others leaves a background with the to-be-removed item as a window shadow (partial remove). I'm baffled. > Cause: > > FLTK's resize mechanism is controlled by the resizable members of the > Group widgets. This is a pointer to the child of the group widget > which the group should resize when its own size changes. The > downloads GUI is buggy because it sets the resizable widget of the > Window to be the PackedGroup that contains the download items, which > is wrong because the PackedGroup is *not* a child of the Window, it's > a child of the the ScrollGroup (which is in turn a child of the > Window). The result is that the contents of the Window don't resize > at all, except when the code tries to work around the bug by > explicitly calling resize(). > > > Fix: > > The code should make the ScrollGroup the resizable widget of the > Window. (The ScrollGroup then properly resizes the PackedGroup > without any further changes. I think this is because its type is set > to VERTICAL.) This makes the width of the ScrollGroup (and hence the > PackedGroup too) follow the width of the Window in a way that is > beautiful to behold. :-) Now we don't need to explicitly resize the > any of the widgets because FLTK does it all automatically whenever the > width of the Window changes, so the call to resize() should be > deleted. -- Cheers Jorge.-

Oct. 30, 2008

Q: D_SUN_LEN , dpi programs and socket programming

by jcid＠dillo.org

On Wed, Oct 29, 2008 at 08:46:28PM +0000, Jeremy Henty wrote: > On Wed, Oct 29, 2008 at 01:28:20PM -0300, Jorge Arellano Cid wrote: > > On Wed, Oct 29, 2008 at 12:24:03PM +0000, Jeremy Henty wrote: > > > > > > Shouldn't we delete the definition of D_SUN_LEN from the dpi > > > programs since they don't use it? Or maybe they should use it? > > > Which? > > > > Given that IO/dpi.c uses both without (reported) problems, it'd be > > good to have everyone use D_SUN_LEN. > > Actually, I think I was confused. The accept() man page says > > The addrlen argument is a value-result argument: it should > initially contain the size of the structure pointed to by addr; on > return it will contain the actual length (in bytes) of the address > returned. > > So passing sizeof(struct sockarr_un) to accept() is correct. Indeed, > passing D_SUN_LUN(&address) could be disastrous because address is > uninitialised (since we expect accept() to fill it in). So I think > the code is correct and we should just remove the unused D_SUN_LEN > definitions. Patch attached. OK, but I was talking about connect(). -- Cheers Jorge.-

Oct. 30, 2008

[patch]: update the GPL version in the openssl exception clause

by onepoint＠starurchin.org

There is a license exception in src/dpi/https.c to link to OpenSSL without following the terms of the GPL. Unfortunately the clause refers to GPL version 2, when everything else has been updated to version 3. Oops! Patch attached. Jeremy Henty

Oct. 30, 2008

[patch]: fix the download GUI resize bug

by onepoint＠starurchin.org

On Wed, Oct 29, 2008 at 01:38:14PM -0300, Jorge Arellano Cid wrote: > On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote: > > > > There's a bug in the downloads GUI that stops the download items > > resizing when the window does. The resizable of the window should > > be the scrollgroup, not the packedgroup. Fixing this means we > > don't need to explicitly resize the scrollgroup after removing an > > item (because the window has already resized it) so I took that > > out. Patch attached. > > Oh, please excuse me not understanding what the bug is and how to > reproduce it. Please explain with apples and pears! :-) OK, fruit 'n' veg coming up! :-) Symptom: If you change the width of the download GUI window its contents do not respond. This is annoying because (a) if you narrow the window to save space then it truncates its contents at the right-hand side rather than gracefully resizing them, and (b) if you widen the window hoping that it will make the display more legible by widening the text labels etc. then nothing happens. It's an easy bug to miss because the result is not obviously wrong, just poor and inconvenient. Once you've seen the correct behaviour the improvement is obvious. Cause: FLTK's resize mechanism is controlled by the resizable members of the Group widgets. This is a pointer to the child of the group widget which the group should resize when its own size changes. The downloads GUI is buggy because it sets the resizable widget of the Window to be the PackedGroup that contains the download items, which is wrong because the PackedGroup is *not* a child of the Window, it's a child of the the ScrollGroup (which is in turn a child of the Window). The result is that the contents of the Window don't resize at all, except when the code tries to work around the bug by explicitly calling resize(). Fix: The code should make the ScrollGroup the resizable widget of the Window. (The ScrollGroup then properly resizes the PackedGroup without any further changes. I think this is because its type is set to VERTICAL.) This makes the width of the ScrollGroup (and hence the PackedGroup too) follow the width of the Window in a way that is beautiful to behold. :-) Now we don't need to explicitly resize the any of the widgets because FLTK does it all automatically whenever the width of the Window changes, so the call to resize() should be deleted. Apples and pears enough? :-) :-) Regards, Jeremy Henty

Oct. 29, 2008

Q: D_SUN_LEN , dpi programs and socket programming

by onepoint＠starurchin.org

On Wed, Oct 29, 2008 at 01:28:20PM -0300, Jorge Arellano Cid wrote: > On Wed, Oct 29, 2008 at 12:24:03PM +0000, Jeremy Henty wrote: > > > > Shouldn't we delete the definition of D_SUN_LEN from the dpi > > programs since they don't use it? Or maybe they should use it? > > Which? > > Given that IO/dpi.c uses both without (reported) problems, it'd be > good to have everyone use D_SUN_LEN. Actually, I think I was confused. The accept() man page says The addrlen argument is a value-result argument: it should initially contain the size of the structure pointed to by addr; on return it will contain the actual length (in bytes) of the address returned. So passing sizeof(struct sockarr_un) to accept() is correct. Indeed, passing D_SUN_LUN(&address) could be disastrous because address is uninitialised (since we expect accept() to fill it in). So I think the code is correct and we should just remove the unused D_SUN_LEN definitions. Patch attached. Jeremy Henty

Oct. 29, 2008

[patch]: fix the download GUI resize bug

by jcid＠dillo.org

On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote: > > There's a bug in the downloads GUI that stops the download items > resizing when the window does. The resizable of the window should be > the scrollgroup, not the packedgroup. Fixing this means we don't need > to explicitly resize the scrollgroup after removing an item (because > the window has already resized it) so I took that out. Patch > attached. Oh, please excuse me not understanding what the bug is and how to reproduce it. Please explain with apples and pears! :-) (yes, I get the explicit resize is strange) -- Cheers Jorge.-

Oct. 29, 2008

Q: D_SUN_LEN , dpi programs and socket programming

by jcid＠dillo.org

On Wed, Oct 29, 2008 at 12:24:03PM +0000, Jeremy Henty wrote: > > The bookmarks and cookies dpis define D_SUN_LEN but don't use it. > D_SUN_LEN is used elsewhere by dillo (when calling connect()) and dpid > (when calling bind()), but all the dpi programs call accept() with > sizeof(struct sockaddr_un) instead of D_SUN_LEN(address). Shouldn't > we delete the definition of D_SUN_LEN from the dpi programs since they > don't use it? Or maybe they should use it? Which? Given that IO/dpi.c uses both without (reported) problems, it'd be good to have everyone use D_SUN_LEN. That way we can filed test easily by changing the macro definition. Maybe there're systems where this still matters. -- Cheers Jorge.-

Oct. 29, 2008

[patch]: fix the download GUI resize bug

by Johannes.Hofmann＠gmx.de

On Wed, Oct 29, 2008 at 02:28:48PM +0000, Jeremy Henty wrote: > > There's a bug in the downloads GUI that stops the download items > resizing when the window does. The resizable of the window should be > the scrollgroup, not the packedgroup. Fixing this means we don't need > to explicitly resize the scrollgroup after removing an item (because > the window has already resized it) so I took that out. Patch > attached. Looks good to me! Cheers, Johannes > > Regards, > > Jeremy Henty > >>From 0e241cbce318768aa8ad8f7715b80dbb3a38a6f3 Mon Sep 17 00:00:00 2001 > From: Jeremy Henty <onepoint(a)starurchin.org> > Date: Wed, 29 Oct 2008 14:14:58 +0000 > Subject: [PATCH] Fix the download GUI resize behaviour. > > --- > dpi/downloads.cc | 5 +---- > 1 files changed, 1 insertions(+), 4 deletions(-) > > diff --git a/dpi/downloads.cc b/dpi/downloads.cc > index d2f3a54..b88deed 100644 > --- a/dpi/downloads.cc > +++ b/dpi/downloads.cc > @@ -1029,9 +1029,6 @@ void DLWin::del(int n_item) > > // Remove the widget from the scroll group > mPG->remove(dl_item->get_widget()); > - // Resize the scroll group > - mPG->resize(mWin->w(), 1); > - > mDList->del(n_item); > delete(dl_item); > } > @@ -1093,7 +1090,7 @@ DLWin::DLWin(int ww, int wh) { > mScroll->end(); > mScroll->type(ScrollGroup::VERTICAL); > mWin->end(); > - mWin->resizable(mPG); > + mWin->resizable(mScroll); > mWin->callback(dlwin_esc_cb, NULL); > mWin->show(); > > -- > 1.6.0.2 > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Oct. 29, 2008

reduce warnings with gcc-4.3 [PATCH]

by Johannes.Hofmann＠gmx.de

On Tue, Oct 28, 2008 at 11:38:25PM +0000, corvid wrote: > Thomas wrote: > > Then at last, there's an unresolved warning: > > > > dillo2-cvs/dw$ LANG=C make textblock.o > > g++ -DHAVE_CONFIG_H -I. -I.. -I/usr/local/include -march=pentium-m -O3 -Wall -W -Wno-unused-parameter -MT textblock.o -MD -MP -MF .deps/textblock.Tpo -c -o textblock.o textblock.cc > > ../lout/misc.hh: In member function 'void dw::Textblock::addLine(int, bool)': > > ../lout/misc.hh:161: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false > > mv -f .deps/textblock.Tpo .deps/textblock.Po > > > > It's about this assert: > > > > inline T* getRef (int i) { > > if (BOUND_CHECKING) > > assert (i >= 0 && i < this->num); > > return array + i; > > } > > > > Ideas to please gcc with this assert? I am not sure what to think of this. > > I was hoping that someone who knows about these things would > reply, but no such luck. Does it matter if you put the > argument into a variable before calling getRef()? > > I don't really see what would be special about one particular > instance of something->getRef(something->size() - n) > when it encountered others earlier without protest, > why it would (seemingly) remember that it was given (this->num - n) > this one time... The problem might be that due to inlining this becomes: assert (this->num - 1 >= 0 && this->num - 1 < this->num); Which looks odd to the compiler. I have no idea how to silence gcc. Perhaps we should just ignore that warning.... Cheers, Johannes

Oct. 29, 2008

[patch]: fix the download GUI resize bug

by onepoint＠starurchin.org

There's a bug in the downloads GUI that stops the download items resizing when the window does. The resizable of the window should be the scrollgroup, not the packedgroup. Fixing this means we don't need to explicitly resize the scrollgroup after removing an item (because the window has already resized it) so I took that out. Patch attached. Regards, Jeremy Henty

Oct. 29, 2008