Hi Jeremy, On Fri, Dec 07, 2007 at 09:02:39PM +0000, Jeremy Henty wrote:
On Fri, Dec 07, 2007 at 06:57:26PM +0000, Jeremy Henty (ie. me) wrote:
* [Johannes Hofmann's] fix for the DeepIterator search methods is the correct fix for the original problem.
* The correct fix for *this* problem is for SelectionState objects to check any DeepIterator objects they create and reset the link/selection if the new DeepIterator object is empty.
OK, I think I have this working. Please test! Johannes' patch for DeepIterator::{searchDownward,searchSideward}() is required. His patch for lout/container.hh is not (I think) required, but I feel it should go in anyway; it would have been nice to have been faced with an obvious NULL rather than memory corruption. Here's the patch to SelectionState::buttonPress() (which also adds DeepIterator::isEmpty() for convenience).
Comments are welcome!
Yes great. Your patch fixes the problem here! Just one thing. I would rather have tested the return value of getTopIterator() for NULL. I think the code is generally a bit optimistic here by assuming non-NULL return value from getTopIterator(). Even if this is now always the case with your fix, it may change again in the future. What do you think, if we add checks for NULL at those three points in the code where getTopIterator() is called, do we still need to check with isEmpty() to call resetLink() / resetSelection() in buttonPress()? Regards, Johannes
Jeremy Henty
Only in dw2-cur: .in Only in dw2-cur: Makefile Only in dw2-cur: Makefile.in Only in dw2-cur: aclocal.m4 Only in dw2-cur: autom4te.cache Only in dw2-cur: config.guess Only in dw2-cur: config.log Only in dw2-cur: config.status Only in dw2-cur: config.sub Only in dw2-cur: configure Only in dw2-cur: depcomp Only in dw2-cur/doc: Makefile Only in dw2-cur/doc: Makefile.in Only in dw2-cur/dw: .deps Only in dw2-cur/dw: Makefile Only in dw2-cur/dw: Makefile.in Only in dw2-cur/dw: findtext.o diff -pru -- dw2-ref/dw/iterator.cc dw2-cur/dw/iterator.cc --- dw2-ref/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 +++ dw2-cur/dw/iterator.cc 2007-12-07 19:07:09.000000000 +0000 @@ -591,6 +591,10 @@ DeepIterator *DeepIterator::createVarian return new DeepIterator (it); }
+bool DeepIterator::isEmpty () { + return ! hasContents; +} + /** * \brief Move iterator forward and store content it. * diff -pru -- dw2-ref/dw/iterator.hh dw2-cur/dw/iterator.hh --- dw2-ref/dw/iterator.hh 2007-10-06 23:03:01.000000000 +0100 +++ dw2-cur/dw/iterator.hh 2007-12-07 19:07:09.000000000 +0000 @@ -171,6 +171,8 @@ public: inline Iterator *getTopIterator () { return stack.getTop(); } inline Content *getContent () { return &content; }
+ bool isEmpty (); + bool next (); bool prev (); inline DeepIterator *cloneDeepIterator() { return (DeepIterator*)clone(); } Only in dw2-cur/dw: iterator.o Only in dw2-cur/dw: layout.o diff -pru -- dw2-ref/dw/selection.cc dw2-cur/dw/selection.cc --- dw2-ref/dw/selection.cc 2007-11-19 15:58:52.000000000 +0000 +++ dw2-cur/dw/selection.cc 2007-12-07 19:08:36.000000000 +0000 @@ -122,15 +122,20 @@ bool SelectionState::buttonPress (Iterat resetLink (); linkState = LINK_PRESSED; linkButton = event->button; - link = new DeepIterator (it); - - // 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; - } + 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; + } + } // We do not return the value of the signal method, // but we do actually process this event. ret = true; @@ -142,10 +147,16 @@ bool SelectionState::buttonPress (Iterat resetSelection ();
selectionState = SELECTING; - from = new DeepIterator (it); - fromChar = correctCharPos (from, charPos); - to = from->cloneDeepIterator (); - toChar = correctCharPos (to, charPos); + 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); + } ret = true; } else { if (event && event->button == 3) {
_______________________________________________ Dillo-dev mailing list Dillo-dev@dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev