- Dillo-dev - lists.mailman3.com

[BUG]: anchors break when a page includes '<meta http-equiv="Content-Type" ...'

by onepoint＠starurchin.org

On Thu, Apr 03, 2008 at 06:16:03PM -0400, Jorge Arellano Cid wrote: > On Fri, Mar 28, 2008 at 05:08:01PM +0000, Jeremy Henty wrote: > > On Fri, Mar 28, 2008 at 09:57:10AM -0400, Jorge Arellano Cid wrote: > > > On Wed, Mar 26, 2008 at 10:23:48PM +0000, Jeremy Henty wrote: > > > > > > > > If the page contains a meta tag that changes the Content-Type > > > > then Dillo re-opens the URL. This calls > > > > Layout::removeWidget(), which in turn calls > > > > Layout::setAnchor(NULL). Ouch! If the URL contained a > > > > fragment then the anchor is lost. Worse, the anchor doesn't > > > > get restored because the URL is flagged as "just reloading" > > > > before it is reopened. The result is that the anchor > > > > mysteriously stops working. > > After a long review of the code, I committed a patch that not only > fixes things in nav.c but in other places which were also involved > in the SEGFAULTs we were getting. Please test it Jeremy. Done. This fixes the anchor-breaking bug that I reported above. No regressions that I can see. Yay! Regards, Jeremy Henty -- If you aren't sure whether you can do this, just try it. The worst thing that can happen is that your computer could explode. -- The GIMP User Manual

April 4, 2008

[BUG]: anchors break when a page includes '<meta http-equiv="Content-Type" ...'

by jcid＠dillo.org

Hi, On Fri, Mar 28, 2008 at 05:08:01PM +0000, Jeremy Henty wrote: > On Fri, Mar 28, 2008 at 09:57:10AM -0400, Jorge Arellano Cid wrote: > > > > On Wed, Mar 26, 2008 at 10:23:48PM +0000, Jeremy Henty wrote: > > > > > > If the page contains a meta tag that changes the Content-Type then > > > Dillo re-opens the URL. This calls Layout::removeWidget(), which > > > in turn calls Layout::setAnchor(NULL). Ouch! If the URL > > > contained a fragment then the anchor is lost. Worse, the anchor > > > doesn't get restored because the URL is flagged as "just > > > reloading" before it is reopened. The result is that the anchor > > > mysteriously stops working. > > > > > > Commenting out the call to Layout::setAnchor(NULL) in > > > Layout::removeWidget() fixes the bug and doesn't seem to break > > > anything else, but I'm not convinced it's the right thing to do. > > > > In fact, repush is an after thought, and some parts of nav also > > need some re-work. The API, IIRC pushing #fragment URLs to the stack > > has some anomalies too. It'd be good to find/design a single path to > > handle it (experience shows multiple paths is asking for trouble :). > > I've looked at it and it seems like deep analysis is required. > > Thanks for explaining this. I'll keep running with > Layout::setAnchor(NULL) commented out for the moment and see how it > goes. (Still no breakage yet.) I hope to be able to spend time > looking at this further. If I have any thoughts about a good way to > do all this I will post them. Unfortunately I have had to devote more > time to earning a living recently (sigh) so I've spent less time > looking at Dillo code than I would have liked. After a long review of the code, I committed a patch that not only fixes things in nav.c but in other places which were also involved in the SEGFAULTs we were getting. Please test it Jeremy. Johannes, there's a patch for dw2 too. It involves minor initialization of scrollX and scrollY, and a printf. If I open dillo with /tmp, a wild value is got for scrollY: jcid@d620:~/C/Dillo/d2/dillo2-cur/src$ ./dillo-fltk /tmp/ dillo_dns_init: Here we go! (threaded) Enabling cookies as from cookiesrc... Nav_open_url: new url='file:/tmp/' Nav_open_url: old_url='' idx=-1 [file dpi]: dpip_tag={<cmd='open_url' url='file:/tmp/' '>} adjustScrollPos: scrollX=0 scrollY=-155 ^^^^--- here! Note: my /tmp is shorter than the viewport. Please give it a look. -- Cheers Jorge.-

April 4, 2008

patch names

by jcid＠dillo.org

Hi, Please submit your patches in this format: diff -pru dillo2 dillo2-cur > Patch-Name-Here.dillo2 diff -pru dw2 dw2-cur > Patch-Name-Here.dw2 -- Cheers Jorge.-

April 3, 2008

popup menus

by jcid＠dillo.org

Hi, On Tue, Apr 01, 2008 at 06:46:34PM +0200, Johannes Hofmann wrote: > Hello, > > On Tue, Apr 01, 2008 at 03:41:29AM +0000, corvid wrote: > > I wrote: > > > I wrote: > > > > Does anybody remember whether the add() call was for > > > > some reason that I haven't noticed? > > I don't think it is necessary as long as Group::current() is NULL > (no begin() without end ()). > Otherwise the popup menu would be added to Group::current() - > whatever that is. I may have added it long ago to "make it work". > > > Hmm. > > > On my various (generally quite simple) pages, it works fine with > > > images or tables or lists or whatever. > > > On some pages out in the wider world, though, it gets > > > really confused. > > > I had thought fltk was getting a mouse event and just going > > > up the fltk widget tree to figure out x,y. That is, just > > > Main -> UI. I guess not. > > > > This makes my problem go away: > > > > diff -pur dw2/dw/fltkui.cc dw2-cur/dw/fltkui.cc > > --- dw2/dw/fltkui.cc 2008-02-05 03:14:22.000000000 +0000 > > +++ dw2-cur/dw/fltkui.cc 2008-04-01 03:24:56.000000000 +0000 > > @@ -972,6 +972,7 @@ template <class I> void FltkSelectionRes > > } > > widgetStack->widget->set_item (pos, widgetStack->stack->size ()); > > } > > + widgetStack->stack->getTop()->getTypedValue()->end (); > > } > > } > > > > Jeremy, does that seem all right? > > Yes, that is right! I would go even further and remove all begin() > calls. They are evil - at least if the matching end () is not > immediately following. > In your case the unmatched end () caused the popup menu to become a > child of some Select-widget that happened to be on the page! From some time Johannes has been warning us about begin-end pairs. I agree with him, they bring side effects (just as the above mentioned one), so removing them favouring explicit add() calls is preferred. Unless they cleanly match each other in the same function, without calling other widget-constructing functions in the middle. > I would propose the attached patch to fltkui.cc that replaces all > begin () calls with direct add () calls. Committed. > Then your proposed removal of the ((Group *)bw->ui)->add(pm) calls > (also attached) seems to work fine. It also makes the annoying > redraws when opening a popup menu go away! Committed. -- Cheers Jorge.-

April 3, 2008

popup menus

by Johannes.Hofmann＠gmx.de

On Tue, Apr 01, 2008 at 06:06:38PM +0000, corvid wrote: > Johannes wrote: > > On Tue, Apr 01, 2008 at 03:41:29AM +0000, corvid wrote: > > > I wrote: > > > > I wrote: > > > > > Does anybody remember whether the add() call was for > > > > > some reason that I haven't noticed? > > > > I don't think it is necessary as long as Group::current() is NULL > > (no begin() without end ()). > > Otherwise the popup menu would be added to Group::current() - > > whatever that is. > > I see that Group.h says > > void end() {current_ = (Group*)parent();} > > so I wonder whether there are still enough begins() and end()s > around that some end() is just setting current to Main for us. No, current_ seems to be NULL now and thereby parent_ of the popup widget is NULL too. But that does not seem to do any harm. (gdb) b menu.cc:263 Breakpoint 1 at 0x806b7bb: file menu.cc, line 263. (gdb) r Starting program: /home/hofmann/projects/dillo/plain/dillo2-hg/src/dillo-fltk dillo_dns_init: Here we go! (threaded) Enabling cookies as from cookiesrc... Fontconfig error: "~/.fonts.conf", line 8: junk after document element Nav_open_url: new url='about:splash' [Switching to process 48228, thread 0x28569000] Breakpoint 1, a_Menu_page_popup (bw=0x285a0a00, url=0x28697580, bugs_txt=0x0, prefs_load_images=1) at menu.cc:266 266 if (bugs_txt == NULL) (gdb) p pm $1 = (class fltk::PopupMenu *) 0x286ec500 (gdb) p *pm $2 = {<fltk::Menu> = {<fltk::Group> = {<fltk::Widget> = {<fltk::Rectangle> = {x_ = 0, y_ = 0, w_ = 0, h_ = 0}, _vptr$Widget = 0x8111440, static default_style = 0x8132fa0, static default_glyph = 0x8130954, label_ = 0x8105d8e "&PAGE OPTIONS", image_ = 0x0, flags_ = 262160, style_ = 0x8131f20, callback_ = 0x80b9778 <fltk::Menu::default_callback(fltk::Widget*, void*)>, user_data_ = 0x0, tooltip_ = 0x0, parent_ = 0x0, type_ = 231 '?', damage_ = 128 '\200', layout_damage_ = 128 '\200', when_ = 4 '\004'}, initial_w = 0, initial_h = 0, children_ = 9, focus_index_ = -1, array_ = 0x287f8600, resizable_ = 0x0, resize_align_ = 15, sizes_ = 0x0, static current_ = 0x0}, static default_style = 0x8131ca0, list_ = 0x8131cfc, item_ = 0x0}, static default_style = 0x8131f20} Cheers, Johannes

April 1, 2008

popup menus

by corvid＠lavabit.com

Johannes wrote: > On Tue, Apr 01, 2008 at 03:41:29AM +0000, corvid wrote: > > I wrote: > > > I wrote: > > > > Does anybody remember whether the add() call was for > > > > some reason that I haven't noticed? > > I don't think it is necessary as long as Group::current() is NULL > (no begin() without end ()). > Otherwise the popup menu would be added to Group::current() - > whatever that is. I see that Group.h says void end() {current_ = (Group*)parent();} so I wonder whether there are still enough begins() and end()s around that some end() is just setting current to Main for us.

April 1, 2008

popup menus

by Johannes.Hofmann＠gmx.de

Hello, On Tue, Apr 01, 2008 at 03:41:29AM +0000, corvid wrote: > I wrote: > > I wrote: > > > Does anybody remember whether the add() call was for > > > some reason that I haven't noticed? I don't think it is necessary as long as Group::current() is NULL (no begin() without end ()). Otherwise the popup menu would be added to Group::current() - whatever that is. > > > > Hmm. > > On my various (generally quite simple) pages, it works fine with > > images or tables or lists or whatever. > > On some pages out in the wider world, though, it gets > > really confused. > > I had thought fltk was getting a mouse event and just going > > up the fltk widget tree to figure out x,y. That is, just > > Main -> UI. I guess not. > > This makes my problem go away: > > diff -pur dw2/dw/fltkui.cc dw2-cur/dw/fltkui.cc > --- dw2/dw/fltkui.cc 2008-02-05 03:14:22.000000000 +0000 > +++ dw2-cur/dw/fltkui.cc 2008-04-01 03:24:56.000000000 +0000 > @@ -972,6 +972,7 @@ template <class I> void FltkSelectionRes > } > widgetStack->widget->set_item (pos, widgetStack->stack->size ()); > } > + widgetStack->stack->getTop()->getTypedValue()->end (); > } > } > > Jeremy, does that seem all right? Yes, that is right! I would go even further and remove all begin() calls. They are evil - at least if the matching end () is not immediately following. In your case the unmatched end () caused the popup menu to become a child of some Select-widget that happened to be on the page! I would propose the attached patch to fltkui.cc that replaces all begin () calls with direct add () calls. Then your proposed removal of the ((Group *)bw->ui)->add(pm) calls (also attached) seems to work fine. It also makes the annoying redraws when opening a popup menu go away! Thanks for your work on this, Johannes > This is yet another case of me not knowing the code at all. > It sure takes a lot of code to construct a web browser... > > > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

April 1, 2008

popup menus

by corvid＠lavabit.com

I wrote: > I wrote: > > Does anybody remember whether the add() call was for > > some reason that I haven't noticed? > > Hmm. > On my various (generally quite simple) pages, it works fine with > images or tables or lists or whatever. > On some pages out in the wider world, though, it gets > really confused. > I had thought fltk was getting a mouse event and just going > up the fltk widget tree to figure out x,y. That is, just > Main -> UI. I guess not. This makes my problem go away: diff -pur dw2/dw/fltkui.cc dw2-cur/dw/fltkui.cc --- dw2/dw/fltkui.cc 2008-02-05 03:14:22.000000000 +0000 +++ dw2-cur/dw/fltkui.cc 2008-04-01 03:24:56.000000000 +0000 @@ -972,6 +972,7 @@ template <class I> void FltkSelectionRes } widgetStack->widget->set_item (pos, widgetStack->stack->size ()); } + widgetStack->stack->getTop()->getTypedValue()->end (); } } Jeremy, does that seem all right? This is yet another case of me not knowing the code at all. It sure takes a lot of code to construct a web browser...

April 1, 2008

popup menus

by corvid＠lavabit.com

I wrote: > Does anybody remember whether the add() call was for > some reason that I haven't noticed? Hmm. On my various (generally quite simple) pages, it works fine with images or tables or lists or whatever. On some pages out in the wider world, though, it gets really confused. I had thought fltk was getting a mouse event and just going up the fltk widget tree to figure out x,y. That is, just Main -> UI. I guess not.

April 1, 2008

(place->corvid) popup menus

by corvid＠lavabit.com

Got back to looking at why popup menus are offset by the height of the control panel, and // Make the popup a child of the calling UI object ((Group *)bw->ui)->add(pm); caught my eye. I thought perhaps if I made them children of Main instead, they would work. But first I tried commenting it out completely -- and now everything seems to work fine. The popups even remember the previous item chosen, which would allow the removal of click-to-load for images. Does anybody remember whether the add() call was for some reason that I haven't noticed? (Jorge cleared the moderation bit for me, so I think you will get this mail!)

March 31, 2008