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! 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) {