Loading src/html.cpp +92 −38 Original line number Diff line number Diff line Loading @@ -665,6 +665,12 @@ void libhtmlpp::HtmlString::_buildTree() { } _buildtreenode(firstEl.get(), nullptr, _rootEl); // _buildtreenode's own linking logic is intricate enough (see its own // comments) that threading parent-assignment through it directly risks // a subtle new bug for marginal benefit -- this single pass over the // now-correctly-linked tree is simple to get right by inspection // instead. if(_rootEl) HtmlElement::_assignParentPointers(_rootEl.get(), nullptr); } /** Loading Loading @@ -757,6 +763,24 @@ const std::string libhtmlpp::HtmlElement::getTagname() const{ return std::string(_TagName.begin(),_TagName.end()); } void libhtmlpp::HtmlElement::_assignParentPointers(Element* node, Element* parent){ while(node){ node->_parentElement = parent; // Only plain HtmlEl nodes use this (base) _childElement for real // children via insertChild/appendChild -- ScriptElement/SvgElement/ // TextArea each shadow their own separate _childElement instead // (and don't support insertChild/appendChild at all, both deleted), // so descending through the base pointer for those types would // read the wrong field entirely. if(node->getType()==HtmlEl){ HtmlElement *hel = static_cast<HtmlElement*>(node); if(hel->_childElement) _assignParentPointers(hel->_childElement.get(), node); } node = node->_nextElement.get(); } } void libhtmlpp::HtmlElement::insertChild(const libhtmlpp::Element* el){ if(_childElement){ remove(_childElement.get()); Loading Loading @@ -784,6 +808,10 @@ void libhtmlpp::HtmlElement::insertChild(const libhtmlpp::Element* el){ throw ex; } _copy(_childElement.get(),el); // _copy re-parents the copied subtree's own descendants (relative to // _childElement.get()) but deliberately never touches _childElement's // OWN parent -- that's this call site's job. _childElement->_parentElement = this; } void libhtmlpp::HtmlElement::insertChild(const Element& el){ Loading Loading @@ -826,6 +854,8 @@ void libhtmlpp::HtmlElement::appendChild(const libhtmlpp::Element* el){ if(prev->_nextElement){ prev->_nextElement->_prevElement=prev; // Siblings share the same parent -- this, not prev. prev->_nextElement->_parentElement=this; } }else{ insertChild(el); Loading Loading @@ -866,41 +896,37 @@ libhtmlpp::HtmlElement & libhtmlpp::HtmlElement::operator=(const libhtmlpp::Html } void libhtmlpp::HtmlElement::remove(libhtmlpp::Element* el){ Element *cur=this; std::stack<Element*> parents; DELETEELEMENT: while(cur){ if(cur==el){ if(cur->_prevElement) cur->_prevElement->_nextElement=std::move(cur->_nextElement); if(cur->_nextElement) cur->_nextElement->_prevElement=cur->_prevElement; cur->_nextElement=nullptr; cur->_prevElement=nullptr; cur=nullptr; } if(cur->getType()==HtmlEl){ if(((HtmlElement*)cur)->_childElement){ parents.push(cur); cur=((HtmlElement*)cur)->_childElement.get(); } } Element *next=cur->_nextElement.get(); cur=next; } if(!parents.empty()){ cur=parents.top()->nextElement(); parents.pop(); if(cur!=el) goto DELETEELEMENT; } // Previously a DFS over the whole subtree via an explicit stack/goto, // searching for whoever owns `el` -- with _parentElement, that lookup // is O(1) instead, and the simpler logic below replaces several bugs // that walk found in that DFS: a null-pointer dereference right after // finding `el` via ordinary sibling advancement (the old code set // `cur=nullptr` on a match and then unconditionally dereferenced it on // the very next line); `el` being silently walked past, never unlinked, // when reached by diving into a `_childElement` (the old code never // re-checked the dived-into node against `el`); and removing a parent's // first child never repointing `parent->_childElement`, which quietly // discarded the rest of that sibling chain. if(!el) return; Element *parent = el->_parentElement; if(parent && parent->getType()==HtmlEl && static_cast<HtmlElement*>(parent)->_childElement.get()==el){ static_cast<HtmlElement*>(parent)->_childElement = std::move(el->_nextElement); if(static_cast<HtmlElement*>(parent)->_childElement) static_cast<HtmlElement*>(parent)->_childElement->_prevElement = nullptr; } else if(el->_prevElement){ Element *next = el->_nextElement.get(); el->_prevElement->_nextElement = std::move(el->_nextElement); if(next) next->_prevElement = el->_prevElement; } // else: el has neither a parent nor a previous sibling (e.g. a // rootless standalone node) -- nothing to unlink it from. el->_prevElement=nullptr; el->_nextElement=nullptr; el->_parentElement=nullptr; } /** Loading Loading @@ -1069,6 +1095,11 @@ namespace libhtmlpp { if(!dest || !src) return; // `dest` itself is reassigned throughout the copy loop below -- // captured here so the parent-pointer fixup at the end can still // find the original top-level destination. libhtmlpp::Element* const originalDest = dest; const libhtmlpp::Element* prev=nullptr; struct cpyel { Loading Loading @@ -1241,6 +1272,21 @@ namespace libhtmlpp { cpylist.pop(); goto NEWEL; } // Re-parent the whole copied subtree relative to originalDest -- // never derived from src's original ancestry, since this copy may // be landing somewhere entirely different (e.g. a fresh standalone // object, or as a new child elsewhere). originalDest's OWN parent // is deliberately left untouched: that's always the caller's // responsibility (insertChild/appendChild set it explicitly right // after calling _copy; operator=/copy-ctors leave it as whatever // it already was, since copying INTO an object doesn't change // where that object itself lives). if(originalDest->getType()==HtmlEl){ libhtmlpp::HtmlElement *destHel = static_cast<libhtmlpp::HtmlElement*>(originalDest); if(destHel->_childElement) libhtmlpp::HtmlElement::_assignParentPointers(destHel->_childElement.get(), originalDest); } return; } }; Loading Loading @@ -1336,6 +1382,10 @@ libhtmlpp::Element *libhtmlpp::Element::prevElement() const{ return _prevElement; } libhtmlpp::Element *libhtmlpp::Element::parentElement() const{ return _parentElement; } libhtmlpp::Element::Element(){ _prevElement=nullptr; _nextElement=nullptr; Loading @@ -1361,11 +1411,15 @@ void libhtmlpp::Element::remove(libhtmlpp::Element* el){ Element *next=curel->_nextElement.get(); if(curel==el){ // `next` (captured above, before the move below) is the // element that ends up adjacent to curel->_prevElement once // curel is spliced out -- checking curel->_nextElement here // instead would always be empty (moved-from) and silently // skip fixing up its _prevElement, leaving it dangling. if(curel->_prevElement) curel->_prevElement->_nextElement=std::move(curel->_nextElement); if(curel->_nextElement) curel->_nextElement->_prevElement=curel->_prevElement; if(next) next->_prevElement=curel->_prevElement; curel->_prevElement=nullptr; curel->_nextElement=nullptr; } Loading src/html.h +11 −0 Original line number Diff line number Diff line Loading @@ -87,6 +87,7 @@ namespace libhtmlpp { Element* nextElement() const; Element* prevElement() const; Element* parentElement() const; virtual void remove(Element* el); Loading @@ -97,6 +98,7 @@ namespace libhtmlpp { std::unique_ptr<Element> _nextElement; Element* _prevElement; Element* _parentElement = nullptr; friend class HtmlElement; friend class TextElement; Loading Loading @@ -180,6 +182,15 @@ namespace libhtmlpp { std::unique_ptr<Attributes> _firstAttr; Attributes* _lastAttr; // Sets `node`'s parent to `parent`, and does the same for every // sibling reachable via nextElement() (they share the same parent) // -- recursing into each HtmlElement-shaped node's own child chain // (with that node as the new parent). Called after building or // copying a subtree, once its own sibling/child links are already // correct, rather than threaded through the parser/copy internals // themselves. static void _assignParentPointers(Element* node, Element* parent); friend class HtmlString; friend class HtmlTable; friend void print(const Element& element, HtmlString &output,bool formated); Loading Loading
src/html.cpp +92 −38 Original line number Diff line number Diff line Loading @@ -665,6 +665,12 @@ void libhtmlpp::HtmlString::_buildTree() { } _buildtreenode(firstEl.get(), nullptr, _rootEl); // _buildtreenode's own linking logic is intricate enough (see its own // comments) that threading parent-assignment through it directly risks // a subtle new bug for marginal benefit -- this single pass over the // now-correctly-linked tree is simple to get right by inspection // instead. if(_rootEl) HtmlElement::_assignParentPointers(_rootEl.get(), nullptr); } /** Loading Loading @@ -757,6 +763,24 @@ const std::string libhtmlpp::HtmlElement::getTagname() const{ return std::string(_TagName.begin(),_TagName.end()); } void libhtmlpp::HtmlElement::_assignParentPointers(Element* node, Element* parent){ while(node){ node->_parentElement = parent; // Only plain HtmlEl nodes use this (base) _childElement for real // children via insertChild/appendChild -- ScriptElement/SvgElement/ // TextArea each shadow their own separate _childElement instead // (and don't support insertChild/appendChild at all, both deleted), // so descending through the base pointer for those types would // read the wrong field entirely. if(node->getType()==HtmlEl){ HtmlElement *hel = static_cast<HtmlElement*>(node); if(hel->_childElement) _assignParentPointers(hel->_childElement.get(), node); } node = node->_nextElement.get(); } } void libhtmlpp::HtmlElement::insertChild(const libhtmlpp::Element* el){ if(_childElement){ remove(_childElement.get()); Loading Loading @@ -784,6 +808,10 @@ void libhtmlpp::HtmlElement::insertChild(const libhtmlpp::Element* el){ throw ex; } _copy(_childElement.get(),el); // _copy re-parents the copied subtree's own descendants (relative to // _childElement.get()) but deliberately never touches _childElement's // OWN parent -- that's this call site's job. _childElement->_parentElement = this; } void libhtmlpp::HtmlElement::insertChild(const Element& el){ Loading Loading @@ -826,6 +854,8 @@ void libhtmlpp::HtmlElement::appendChild(const libhtmlpp::Element* el){ if(prev->_nextElement){ prev->_nextElement->_prevElement=prev; // Siblings share the same parent -- this, not prev. prev->_nextElement->_parentElement=this; } }else{ insertChild(el); Loading Loading @@ -866,41 +896,37 @@ libhtmlpp::HtmlElement & libhtmlpp::HtmlElement::operator=(const libhtmlpp::Html } void libhtmlpp::HtmlElement::remove(libhtmlpp::Element* el){ Element *cur=this; std::stack<Element*> parents; DELETEELEMENT: while(cur){ if(cur==el){ if(cur->_prevElement) cur->_prevElement->_nextElement=std::move(cur->_nextElement); if(cur->_nextElement) cur->_nextElement->_prevElement=cur->_prevElement; cur->_nextElement=nullptr; cur->_prevElement=nullptr; cur=nullptr; } if(cur->getType()==HtmlEl){ if(((HtmlElement*)cur)->_childElement){ parents.push(cur); cur=((HtmlElement*)cur)->_childElement.get(); } } Element *next=cur->_nextElement.get(); cur=next; } if(!parents.empty()){ cur=parents.top()->nextElement(); parents.pop(); if(cur!=el) goto DELETEELEMENT; } // Previously a DFS over the whole subtree via an explicit stack/goto, // searching for whoever owns `el` -- with _parentElement, that lookup // is O(1) instead, and the simpler logic below replaces several bugs // that walk found in that DFS: a null-pointer dereference right after // finding `el` via ordinary sibling advancement (the old code set // `cur=nullptr` on a match and then unconditionally dereferenced it on // the very next line); `el` being silently walked past, never unlinked, // when reached by diving into a `_childElement` (the old code never // re-checked the dived-into node against `el`); and removing a parent's // first child never repointing `parent->_childElement`, which quietly // discarded the rest of that sibling chain. if(!el) return; Element *parent = el->_parentElement; if(parent && parent->getType()==HtmlEl && static_cast<HtmlElement*>(parent)->_childElement.get()==el){ static_cast<HtmlElement*>(parent)->_childElement = std::move(el->_nextElement); if(static_cast<HtmlElement*>(parent)->_childElement) static_cast<HtmlElement*>(parent)->_childElement->_prevElement = nullptr; } else if(el->_prevElement){ Element *next = el->_nextElement.get(); el->_prevElement->_nextElement = std::move(el->_nextElement); if(next) next->_prevElement = el->_prevElement; } // else: el has neither a parent nor a previous sibling (e.g. a // rootless standalone node) -- nothing to unlink it from. el->_prevElement=nullptr; el->_nextElement=nullptr; el->_parentElement=nullptr; } /** Loading Loading @@ -1069,6 +1095,11 @@ namespace libhtmlpp { if(!dest || !src) return; // `dest` itself is reassigned throughout the copy loop below -- // captured here so the parent-pointer fixup at the end can still // find the original top-level destination. libhtmlpp::Element* const originalDest = dest; const libhtmlpp::Element* prev=nullptr; struct cpyel { Loading Loading @@ -1241,6 +1272,21 @@ namespace libhtmlpp { cpylist.pop(); goto NEWEL; } // Re-parent the whole copied subtree relative to originalDest -- // never derived from src's original ancestry, since this copy may // be landing somewhere entirely different (e.g. a fresh standalone // object, or as a new child elsewhere). originalDest's OWN parent // is deliberately left untouched: that's always the caller's // responsibility (insertChild/appendChild set it explicitly right // after calling _copy; operator=/copy-ctors leave it as whatever // it already was, since copying INTO an object doesn't change // where that object itself lives). if(originalDest->getType()==HtmlEl){ libhtmlpp::HtmlElement *destHel = static_cast<libhtmlpp::HtmlElement*>(originalDest); if(destHel->_childElement) libhtmlpp::HtmlElement::_assignParentPointers(destHel->_childElement.get(), originalDest); } return; } }; Loading Loading @@ -1336,6 +1382,10 @@ libhtmlpp::Element *libhtmlpp::Element::prevElement() const{ return _prevElement; } libhtmlpp::Element *libhtmlpp::Element::parentElement() const{ return _parentElement; } libhtmlpp::Element::Element(){ _prevElement=nullptr; _nextElement=nullptr; Loading @@ -1361,11 +1411,15 @@ void libhtmlpp::Element::remove(libhtmlpp::Element* el){ Element *next=curel->_nextElement.get(); if(curel==el){ // `next` (captured above, before the move below) is the // element that ends up adjacent to curel->_prevElement once // curel is spliced out -- checking curel->_nextElement here // instead would always be empty (moved-from) and silently // skip fixing up its _prevElement, leaving it dangling. if(curel->_prevElement) curel->_prevElement->_nextElement=std::move(curel->_nextElement); if(curel->_nextElement) curel->_nextElement->_prevElement=curel->_prevElement; if(next) next->_prevElement=curel->_prevElement; curel->_prevElement=nullptr; curel->_nextElement=nullptr; } Loading
src/html.h +11 −0 Original line number Diff line number Diff line Loading @@ -87,6 +87,7 @@ namespace libhtmlpp { Element* nextElement() const; Element* prevElement() const; Element* parentElement() const; virtual void remove(Element* el); Loading @@ -97,6 +98,7 @@ namespace libhtmlpp { std::unique_ptr<Element> _nextElement; Element* _prevElement; Element* _parentElement = nullptr; friend class HtmlElement; friend class TextElement; Loading Loading @@ -180,6 +182,15 @@ namespace libhtmlpp { std::unique_ptr<Attributes> _firstAttr; Attributes* _lastAttr; // Sets `node`'s parent to `parent`, and does the same for every // sibling reachable via nextElement() (they share the same parent) // -- recursing into each HtmlElement-shaped node's own child chain // (with that node as the new parent). Called after building or // copying a subtree, once its own sibling/child links are already // correct, rather than threaded through the parser/copy internals // themselves. static void _assignParentPointers(Element* node, Element* parent); friend class HtmlString; friend class HtmlTable; friend void print(const Element& element, HtmlString &output,bool formated); Loading