- Dillo-dev - lists.mailman3.com

[PATCH] DW: indentation in dw/selection.cc
by onepoint＠starurchin.org Dec. 12, 2007

Dec. 12, 2007

This is purely cosmetic: my changes to dw/selection.cc messed up the indentation so this patch fixes it. Incidentally, does anyone have a version of the emacs C/C++ modes that is consistent with the Dillo source? I've set: c-basic offset: 3 indent-tabs-mode: nil but it still pointlessly reformats the source. In particular it indents the contents of namespace { ... }. Regards, Jeremy Henty diff -pru -- dw2-ref/dw/selection.cc dw2-cur/dw/selection.cc --- dw2-ref/dw/selection.cc 2007-12-11 11:30:07.000000000 +0000 +++ dw2-cur/dw/selection.cc 2007-12-11 20:54:10.000000000 +0000 @@ -123,19 +123,19 @@ bool SelectionState::buttonPress (Iterat linkState = LINK_PRESSED; linkButton = event->button; DeepIterator *newLink = new DeepIterator (it); - if (newLink->isEmpty ()) { - delete newLink; - resetLink (); - } else { - link = newLink; - // It may be that the user has pressed on something activatable - // (linkNo != -1), but there is no contents, e.g. with images - // without ALTernative text. - if (link) { - linkChar = correctCharPos (link, charPos); - linkNumber = linkNo; - } - } + if (newLink->isEmpty ()) { + delete newLink; + resetLink (); + } else { + link = newLink; + // It may be that the user has pressed on something activatable + // (linkNo != -1), but there is no contents, e.g. with images + // without ALTernative text. + if (link) { + linkChar = correctCharPos (link, charPos); + linkNumber = linkNo; + } + } // We do not return the value of the signal method, // but we do actually process this event. ret = true; @@ -148,15 +148,15 @@ bool SelectionState::buttonPress (Iterat selectionState = SELECTING; DeepIterator *newFrom = new DeepIterator (it); - if (newFrom->isEmpty ()) { - delete newFrom; - resetSelection (); - } else { - from = newFrom; - fromChar = correctCharPos (from, charPos); - to = from->cloneDeepIterator (); - toChar = correctCharPos (to, charPos); - } + if (newFrom->isEmpty ()) { + delete newFrom; + resetSelection (); + } else { + from = newFrom; + fromChar = correctCharPos (from, charPos); + to = from->cloneDeepIterator (); + toChar = correctCharPos (to, charPos); + } ret = true; } else { if (event && event->button == 3) {

2 1

BUG: assertion failed: "no widget": in dw::core::DeepIterator::searchSideward
by onepoint＠starurchin.org Dec. 11, 2007

Dec. 11, 2007

Dillo aborts with an assertion failure if you click to the right of an image that has a line all to itself. For example, go to http://www.thenation.com/doc/20071203/pollitt and click to the right of the "Continued" logo. What happens is that the DeepIterator constructor wants an iterator with non-widget content. If necessary it calls searchDownward and searchSideward until it finds one. Both these methods raise an assertion failure if the iterator already has non-widget content, so the content must be checked before calling *either* of them. However, DeepIterator checks the content before calling searchDownward but *not* before calling searchSideward . Effectively, the code assumes that if searchDownward fails then the content type won't have changed. Unfortunately searchDownward can fail *and* change the content of the iterator to something non-widget. If this happens DeepIterator then calls searchSideward, which triggers the assertion. (NB: DeepIterator is in the dw::core namespace.) I'm trying to fix this but so far I'm just generating more crashes. What exactly should DeepIterator::DeepIterator do if searchDownward fails and changes the content of the iterator? If I know that I'll have another go. Regards, Jeremy Henty

3 14

[patch] fix close button height in page source window
by Johannes.Hofmann＠gmx.de Dec. 11, 2007

Dec. 11, 2007

Hi, the height of the close button of the source code window changes with the size of the window. The patch below fixes its size. Cheers, Johannes PS: Scrolling in the page source window is dead slow here. But that's definately a fltk issue... diff -r 043b53a25dc3 src/dialog.cc --- a/src/dialog.cc Mon Dec 10 15:42:03 2007 +0100 +++ b/src/dialog.cc Mon Dec 10 18:12:15 2007 +0100 @@ -123,7 +123,7 @@ void a_Dialog_text_window(const char *tx ReturnButton *b = new ReturnButton (0, wh-bh, ww, bh, "Close"); b->callback(window_close_cb, window); - window->resizable(window); + window->resizable(td); window->end(); window->show(); }

2 1

BUG: CCC corrupts memory when the Content-Type is wrong
by onepoint＠starurchin.org Dec. 6, 2007

Dec. 6, 2007

I am hoping that someone who understands Dillo's CCC better than I can point me in the right direction and save me some time debugging this. Dillo almost always crashes if the HTML contains the stumbleupon.com favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. (This is very annoying as it makes Dillo unusable for one of my favourite blogs.) If it doesn't crash right away then clicking Reload crashes it. Examining core dumps with gdb suggests that memory is being corrupted, and running dillo under valgrind shows that the CCC functions are reading, writing and freeing already freed memory. (Details attached.) A local HTTP server triggers the bug when it server returns a Content-Type of "text/plain; charset=iso-8859-1". If I change it to "image/x-icon" there is no corruption. Accessing the image directly via a "file:" URL does not trigger the bug. (Strangely, using a local HTTP server does not hit the bug until I open a second window on the page, although sometimes it crashes first time.) I'm attaching a summary of the invalid memory accesses that valgrind reported, and the entire valgrind log (which includes stack traces). Any suggestions for where I should start instrumenting the CCC to find out why it is prematurely freeing pointers? Jeremy Henty

2 4

[patch] fix crashes after panel size changes
by Johannes.Hofmann＠gmx.de Dec. 6, 2007

Dec. 6, 2007

Hello, now that was a weird bug! I was experiencing crashes when I had changed the panel size via the right mouse on the search button. The problem is that FltkWidgetView::prepareCreateFltkWidget() was calling fltk::Group::begin(). Therefore after visiting pages with forms, the newly created panel was getting a child of Main instead of TopGroup! Btw. did I mention that this fltk begin()/end() stuff is evil? One could expect that Group::replace() should disconnect the new widget properly from its old parent (if there is any). But that's not the case. I wrote a mail to fltk.general about this. So in the end Main still had a reference to the panel and once it was deleted -- bang. The patch below uses Group::add() directly in FltkWidgetView::addFltkWidget() instead of FltkWidgetView::prepareCreateFltkWidget(). Does anyone remember why FltkWidgetView::prepareCreateFltkWidget() was added in the first place? Cheers, Johannes diff --git a/dw/fltkplatform.cc b/dw/fltkplatform.cc --- a/dw/fltkplatform.cc +++ b/dw/fltkplatform.cc @@ -83,10 +83,6 @@ FltkColor::FltkColor (int color, core::s colors[SHADING_DARK] = shadeColor (color, SHADING_DARK) << 8; colors[SHADING_LIGHT] = shadeColor (color, SHADING_LIGHT) << 8; } -} - -void FltkView::prepareCreateFltkWidget () -{ } void FltkView::addFltkWidget (::fltk::Widget *widget, diff --git a/dw/fltkplatform.hh b/dw/fltkplatform.hh --- a/dw/fltkplatform.hh +++ b/dw/fltkplatform.hh @@ -40,7 +40,6 @@ public: public: virtual bool usesFltkWidgets () = 0; - virtual void prepareCreateFltkWidget (); virtual void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); virtual void removeFltkWidget (::fltk::Widget *widget); diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -103,7 +103,6 @@ void FltkResource::attachView (FltkView ViewAndWidget *viewAndWidget = new ViewAndWidget(); viewAndWidget->view = view; - view->prepareCreateFltkWidget (); viewAndWidget->widget = createNewWidget (&allocation); viewAndWidget->view->addFltkWidget (viewAndWidget->widget, &allocation); viewsAndWidgets->append (viewAndWidget); diff --git a/dw/fltkviewbase.cc b/dw/fltkviewbase.cc --- a/dw/fltkviewbase.cc +++ b/dw/fltkviewbase.cc @@ -375,17 +375,12 @@ bool FltkWidgetView::usesFltkWidgets () return true; } - -void FltkWidgetView::prepareCreateFltkWidget () -{ - begin (); -} - void FltkWidgetView::addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation) { canvasWidgets->append (new TypedPointer < ::fltk::Widget> (widget)); allocateFltkWidget (widget, allocation); + add (widget); } void FltkWidgetView::removeFltkWidget (::fltk::Widget *widget) diff --git a/dw/fltkviewbase.hh b/dw/fltkviewbase.hh --- a/dw/fltkviewbase.hh +++ b/dw/fltkviewbase.hh @@ -82,7 +82,6 @@ public: int x, int y, int width, int height); bool usesFltkWidgets (); - void prepareCreateFltkWidget (); void addFltkWidget (::fltk::Widget *widget, core::Allocation *allocation); void removeFltkWidget (::fltk::Widget *widget); void allocateFltkWidget (::fltk::Widget *widget,

2 1

[patch] initialize mustQueueResize in Textblock constructor
by Johannes.Hofmann＠gmx.de Dec. 5, 2007

Dec. 5, 2007

Hello, this patch initializes mustQueueResize in the Textblock constructor. Valgrind was complaining about a jump depending on uninitialized memory. Cheers, Johannes diff --git a/dw/textblock.cc b/dw/textblock.cc --- a/dw/textblock.cc +++ b/dw/textblock.cc @@ -39,6 +39,7 @@ Textblock::Textblock (bool limitTextWidt line1Offset = 0; line1OffsetEff = 0; ignoreLine1OffsetSometimes = false; + mustQueueResize = false; /* * The initial sizes of lines and words should not be

2 1

[patch] add missing style->ref ()
by Johannes.Hofmann＠gmx.de Dec. 5, 2007

Dec. 5, 2007

Hello, the tiny patch below adds a missing style->ref() which was causing a lot of crashes on my system. cheers, Johannes diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -159,6 +159,7 @@ void FltkResource::setStyle (core::style this->style->unref (); this->style = style; + style->ref (); for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); it.hasNext(); ) {

2 1

[patch] partial redraws - updated
by Johannes.Hofmann＠gmx.de Dec. 4, 2007

Dec. 4, 2007

Hello, this patch fixes the problem pointed out by Sebastian regarding partial redraws and scrolling interference. It adds a new Region class that is implemented similarily as in dillo1 as a set of rectangles not containing each other. The conversion from canvas to view coordinates is now done at draw-time. Regards, Johannes PS: The code did not get as complex as I initially thought it would :-)

2 5

Re: patch: a _minuscule_ amount of cleaning up the header parsing code
by jcid＠dillo.org Dec. 3, 2007

Dec. 3, 2007

On Thu, Nov 29, 2007 at 03:08:12PM +0000, place wrote: > No change in behavior is intended. > Yes, that code is getting hard to understand. After a review, I wondered why Cache_parse_header() is dealing with the message body, when it should only parse the header, and why is it getting the (buf, bufsize) pair when it has the content in entry->Header. It looks that a cleaner way would be something like: (not tested, just pseudo-code) if (!(entry->Flags & CA_GotHeader)) { while ((len = Cache_get_header(entry, buf + start, buf_size - start))) { /* Let's scan, allocate, and set things according to header info */ Cache_parse_header(entry); if (entry->Flags & CA_GotHeader) { start += len; } else // Let the flow continue downward so the remaining // of the buffer is processed } return; } and to get rid of the: dbuf = a_Decode_process(entry->TransferDecoder, dbuf); dbuf = a_Decode_process(entry->ContentDecoder, dbuf); lines inside Cache_parse_header(). Please give it a try. -- Cheers Jorge.-

1 0

Re: charset conversion issues
by jcid＠dillo.org Nov. 28, 2007

Nov. 28, 2007

Hi, On Sat, Nov 24, 2007 at 10:50:18PM +0000, place wrote: > With rough code to look at. > No sense polishing up and completing what I have yet when you'll want changes... OK. > > > Charset in the meta tag. > My options seemed to be: > - Client->Callback returns information to cache. > Probably not supposed to happen. Too big of a change. Agreed. At least to postpone until other approaches prove messier. ;-) > - Reach the meta tag and undo/redo within html.cc. > Somewhat fragile. > Whatever I might come up with, some html out there would surely find a way > to outsmart me. This is the one I like most at first sight. Considering the charset can be given by HTTP or a META element: We can assume ASCII in the html text until the HEAD element is closed. If there's a charset in the META, then the decoder can be switched from null to the specified one. This approach has the advantage of working both when the charset comes via HTTP or META (<HEAD> content is ASCII). We can even add a text buffer for the HEAD element and append it to the whole HTML content if the offset is hard to set for the new decoder. Do the problems you found apply to this scheme too? > - Bail out and start again. > This is what I have now. In fact, not knowing at all what to do, I just > call a_Nav_cancel_expect() and a_Nav_push() right then and there. > Surprisingly, it functions instead of crashing, but I'm probably spilling > resources And Violating Principles. > (And, since a_Nav_push() isn't quite right for the situation even then, > I have to fix the nav stack ptr in Nav_cleanup()) > > > There's a dialog box to set the Content-Type for a page. If you change it > to text/plain, you get source in a proper window (with searching) instead > of the little fltk window. No line numbers, though. The idea of using Dillo for view source is interesting, i'm not sure though. Maybe a fltk widget (like an stripped-down editor as the one in FLTK's test/ directory) which gets utf-8 via a_Capi_get_utf8_data(Url) is also good. > > > Not wanting to mess with TypeDet and TypeHdr until finding out what you > think, I stuck in another one, TypeSet. I'd prefer to try to apply the second method (described above). > I'm directly calling a_Cache_[sg]et_content_type() from just wherever, > which may or may not be all right. Cache should be called from Capi only, but at this stage... > I knew that latin1 != UTF-8, but I was not thinking when I wrote > that Decode code. Must've been thinking about the Unicode code numbers. > FLTK has a workaround in it to deal with fools like me who send it latin1 > or cp1252, but better that I stop being foolish :) > > > I don't have Plain or View Source set up to decode yet. I don't know whether > the cache should know about decoding charsets. Or a Text base class or > something. > > > The real-type-matches-header-type code will have to learn to relax a bit, > now that html might contain all kinds of bytes... -- Cheers Jorge.-

2 1