Skip to content

Commit e42f93b

Browse files
michaelDCurranmichaelDCurran
committed
VBufStorage and backends: Ensure that reuse is denied when previous siblings have changed, only if a node's denyReuseIfPreviousSiblingsChanged property is actually set (woops). Also, remove ensureDescendantsRequireParentUpdate method and replace it with a alwaysRerenderDescendants property instead.
1 parent 4061173 commit e42f93b

5 files changed

Lines changed: 38 additions & 51 deletions

File tree

‎nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,11 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
780780
if (!nameIsContent && name)
781781
parentNode->addAttribute(L"name", name);
782782

783+
if(nameIsContent) {
784+
// We may render an accessible name for this node if it has been explicitly set or it has no useful content.
785+
parentNode->alwaysRerenderDescendants=true;
786+
}
787+
783788
if (isVisible) {
784789
if ( isImgMap && name ) {
785790
// This is an image map with a name. Render the name first.
@@ -926,26 +931,20 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
926931
// If a link has no name, derive it from the URL.
927932
buffer->addTextFieldNode(parentNode, NULL, getNameForURL(value));
928933
}
929-
// If any descendant is invalidated, this may change whether this node has no useful content.
930-
parentNode->ensureDescendantsRequireParentUpdate();
931934
}
932935

933936
if ((role == ROLE_SYSTEM_CELL || role == ROLE_SYSTEM_ROWHEADER || role == ROLE_SYSTEM_COLUMNHEADER||role==IA2_ROLE_UNKNOWN) && parentNode->getLength() == 0) {
934937
// Always render a space for empty table cells and unknowns.
935938
previousNode=buffer->addTextFieldNode(parentNode,previousNode,L" ");
936939
if(previousNode&&!locale.empty()) previousNode->addAttribute(L"language",locale);
937940
parentNode->isBlock=false;
938-
// If any descendant is invalidated, this may change whether this node has no content.
939-
parentNode->ensureDescendantsRequireParentUpdate();
940941
}
941942

942943
if ((isInteractive || role == ROLE_SYSTEM_SEPARATOR) && parentNode->getLength() == 0) {
943944
// If the node is interactive or otherwise relevant even when empty
944945
// and it still has no content, render a space so the user can access the node.
945946
previousNode=buffer->addTextFieldNode(parentNode,previousNode,L" ");
946947
if(previousNode&&!locale.empty()) previousNode->addAttribute(L"language",locale);
947-
// If any descendant is invalidated, this may change whether this node has no useful content.
948-
parentNode->ensureDescendantsRequireParentUpdate();
949948
}
950949
}
951950

‎nvdaHelper/vbufBackends/mshtml/mshtml.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@ if(!(formatState&FORMATSTATE_INSERTED)&&nodeName.compare(L"INS")==0) {
12451245
fillTextFormattingForNode(pHTMLDOMNode,previousNode);
12461246
}
12471247
// If any descendant is invalidated, this may change whether this node has no useful content.
1248-
parentNode->ensureDescendantsRequireParentUpdate();
1248+
parentNode->alwaysRerenderDescendants=true;
12491249
}
12501250
}
12511251

@@ -1280,16 +1280,12 @@ if(!(formatState&FORMATSTATE_INSERTED)&&nodeName.compare(L"INS")==0) {
12801280
if(parentNode->getLength()==0) {
12811281
isBlock=false;
12821282
buffer->addTextFieldNode(parentNode,previousNode,L" ");
1283-
// If any descendant is invalidated, this may change whether this node has no useful content.
1284-
parentNode->ensureDescendantsRequireParentUpdate();
12851283
}
12861284
}
12871285

12881286
//If a node is interactive, and still has no content, add a space
12891287
if(isInteractive&&parentNode->getLength()==0) {
12901288
buffer->addTextFieldNode(parentNode,previousNode,L" ");
1291-
// If any descendant is invalidated, this may change whether this node has no useful content.
1292-
parentNode->ensureDescendantsRequireParentUpdate();
12931289
}
12941290
}
12951291

‎nvdaHelper/vbufBase/backend.cpp‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ void VBufBackend_t::update() {
199199
//render all invalid subtrees, storing each subtree in its own buffer
200200
for(auto i=workingInvalidSubtreesList.begin();i!=workingInvalidSubtreesList.end();) {
201201
VBufStorage_controlFieldNode_t* node=*i;
202-
LOG_DEBUG(L"re-rendering subtree at "<<node);
202+
LOG_DEBUG(L"re-rendering subtree from "<<node->getDebugInfo());
203203
VBufStorage_buffer_t* tempBuf=new VBufStorage_buffer_t();
204204
nhAssert(tempBuf); //tempBuf can't be NULL
205205
LOG_DEBUG(L"Created temp buffer at "<<tempBuf);
@@ -262,7 +262,7 @@ VBufBackend_t::~VBufBackend_t() {
262262

263263
VBufStorage_controlFieldNode_t* VBufBackend_t::reuseExistingNodeInRender(VBufStorage_controlFieldNode_t* parent, VBufStorage_fieldNode_t* previous, int docHandle, int ID) {
264264
LOG_DEBUG(L"Try to reuse node with docHandle "<<docHandle<<L", and ID "<<ID);
265-
if(parent->alwaysRerenderChildren) {
265+
if(parent->alwaysRerenderDescendants||parent->alwaysRerenderChildren) {
266266
LOG_DEBUG(L"Won't find a node to reuse as parent says always rerender children");
267267
return nullptr;
268268
}
@@ -277,27 +277,29 @@ VBufStorage_controlFieldNode_t* VBufBackend_t::reuseExistingNodeInRender(VBufSto
277277
LOG_DEBUG(L"Existing node refuses to be reused");
278278
return nullptr;
279279
}
280-
VBufStorage_controlFieldNode_t* previousControlFieldNode=nullptr;
281-
for(auto tempNode=previous;tempNode!=nullptr;tempNode=tempNode->getPrevious()) {
282-
previousControlFieldNode=dynamic_cast<VBufStorage_controlFieldNode_t*>(tempNode);
283-
if(previousControlFieldNode) break;
284-
}
285-
VBufStorage_referenceNode_t* previousReferenceNode=dynamic_cast<VBufStorage_referenceNode_t*>(previousControlFieldNode);
286-
if(previousControlFieldNode&&!previousReferenceNode) {
287-
LOG_DEBUG(L"Previous controlFieldNode was not a referenceNode");
288-
return nullptr;
289-
}
290-
if(previousReferenceNode) {
291-
previousControlFieldNode=previousReferenceNode->referenceNode;
292-
}
293-
VBufStorage_controlFieldNode_t* previousExistingControlFieldNode=nullptr;
294-
for(auto tempNode=existingNode->getPrevious();tempNode!=nullptr;tempNode=tempNode->getPrevious()) {
295-
previousExistingControlFieldNode=dynamic_cast<VBufStorage_controlFieldNode_t*>(tempNode);
296-
if(previousExistingControlFieldNode) break;
297-
}
298-
if(previousControlFieldNode!=previousExistingControlFieldNode) {
299-
LOG_DEBUG(L"Previous controlFieldNodes differ");
300-
return nullptr;
280+
if(existingNode->denyReuseIfPreviousSiblingsChanged) {
281+
VBufStorage_controlFieldNode_t* previousControlFieldNode=nullptr;
282+
for(auto tempNode=previous;tempNode!=nullptr;tempNode=tempNode->getPrevious()) {
283+
previousControlFieldNode=dynamic_cast<VBufStorage_controlFieldNode_t*>(tempNode);
284+
if(previousControlFieldNode) break;
285+
}
286+
VBufStorage_referenceNode_t* previousReferenceNode=dynamic_cast<VBufStorage_referenceNode_t*>(previousControlFieldNode);
287+
if(previousControlFieldNode&&!previousReferenceNode) {
288+
LOG_DEBUG(L"Previous controlFieldNode was not a referenceNode");
289+
return nullptr;
290+
}
291+
if(previousReferenceNode) {
292+
previousControlFieldNode=previousReferenceNode->referenceNode;
293+
}
294+
VBufStorage_controlFieldNode_t* previousExistingControlFieldNode=nullptr;
295+
for(auto tempNode=existingNode->getPrevious();tempNode!=nullptr;tempNode=tempNode->getPrevious()) {
296+
previousExistingControlFieldNode=dynamic_cast<VBufStorage_controlFieldNode_t*>(tempNode);
297+
if(previousExistingControlFieldNode) break;
298+
}
299+
if(previousControlFieldNode!=previousExistingControlFieldNode) {
300+
LOG_DEBUG(L"Previous controlFieldNodes differ");
301+
return nullptr;
302+
}
301303
}
302304
auto i=std::find(this->workingInvalidSubtreesList.begin(),this->workingInvalidSubtreesList.end(),existingNode);
303305
if(i!=this->workingInvalidSubtreesList.end()) {

‎nvdaHelper/vbufBase/storage.cpp‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,17 +347,6 @@ VBufStorage_controlFieldNode_t::VBufStorage_controlFieldNode_t(int docHandle, in
347347
LOG_DEBUG(L"controlFieldNode initialization at "<<this<<L", with docHandle of "<<identifier.docHandle<<L" and ID of "<<identifier.ID);
348348
}
349349

350-
void VBufStorage_controlFieldNode_t::ensureDescendantsRequireParentUpdate() {
351-
this->alwaysRerenderChildren=true;
352-
for(auto child=this->getFirstChild();child!=nullptr;child=child->getNext()) {
353-
auto childControlField=dynamic_cast<VBufStorage_controlFieldNode_t*>(child);
354-
if(childControlField) {
355-
childControlField->requiresParentUpdate=true;
356-
childControlField->ensureDescendantsRequireParentUpdate();
357-
}
358-
}
359-
}
360-
361350
bool VBufStorage_controlFieldNode_t::getIdentifier(int* docHandle, int* ID) {
362351
*docHandle=this->identifier.docHandle;
363352
*ID=this->identifier.ID;
@@ -558,6 +547,10 @@ VBufStorage_controlFieldNode_t* VBufStorage_buffer_t::addControlFieldNode(VBufS
558547
return NULL;
559548
}
560549
controlFieldNodesByIdentifier[controlFieldNode->identifier]=controlFieldNode;
550+
// If the node's new parent requires descendants to always be rerendered, copy this etting to the node as well.
551+
if(parent&&parent->alwaysRerenderDescendants) {
552+
controlFieldNode->alwaysRerenderDescendants=true;
553+
}
561554
LOG_DEBUG(L"Added new controlFieldNode, returning node");
562555
return controlFieldNode;
563556
}

‎nvdaHelper/vbufBase/storage.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,13 +357,10 @@ class VBufStorage_controlFieldNode_t : public VBufStorage_fieldNode_t {
357357
*/
358358
bool alwaysRerenderChildren {false};
359359

360-
/**
361-
* Sets the requiresParentUpdate property to true on all descendants of this node, and sets alwaysRerenderChildren to true on this node and all descendants.
362-
* This method is useful if some alternative text has been added to a node because its descendants don't seem to contain useful content (E.g. an empty link).
363-
* This ensures that if a descendant is re-rendered, this node should also be re-rendered (as the alternative text may no longer be needed).
364-
* It also ensures that if this node is re-rendered, none of its children are reused from here or somewhere else in the tree as the reused node may not have been makred as requiring a prent update.
360+
/*
361+
* If true, all this node's descendants will always be re-rendered along with this node when being re-rendered.
365362
*/
366-
void ensureDescendantsRequireParentUpdate();
363+
bool alwaysRerenderDescendants {false};
367364

368365
/**
369366
* retreaves the node's doc handle and ID.

0 commit comments

Comments
 (0)