Crates: only store or activate sibling crate if it's valid #11770
Conversation
| @@ -877,7 +880,9 @@ void CrateFeature::slotCrateTableChanged(CrateId crateId) { | |||
| if (!activateCrate(m_crateTableModel.selectedCrate())) { | |||
| // probably last clicked crate was deleted, try to | |||
| // select the stored sibling | |||
| activateCrate(m_prevSiblingCrate); | |||
| if (m_prevSiblingCrate.isValid()) { | |||
| activateCrate(m_prevSiblingCrate); | |||
There was a problem hiding this comment.
This seems to be the fix, right?
There was a problem hiding this comment.
This code is also in place at 2.3.
Does this crash happen in 2.3 as well?
| @@ -862,7 +862,10 @@ void CrateFeature::storePrevSiblingCrateId(CrateId crateId) { | |||
| TreeItem* pTreeItem = m_pSidebarModel->getItem(newIndex); | |||
| DEBUG_ASSERT(pTreeItem != nullptr); | |||
| if (!pTreeItem->hasChildren()) { | |||
| m_prevSiblingCrate = crateIdFromIndex(newIndex); | |||
| CrateId prevCrateId = crateIdFromIndex(newIndex); | |||
| if (prevCrateId.isValid()) { | |||
There was a problem hiding this comment.
I did a look around and I have not yet understand in which cases it can be invalid at all? Does this case source from another bug deeper in the code? Can we have children with invalid crates? Please add a comment to the code.
Idea: The loop can be reversed and exited early with break. That would make the "search for one sibling" nature more clear.
There was a problem hiding this comment.
True, this is excessive. The check in activateCrate() is sufficient.
I'll revert this.
ronso0
force-pushed
the
crate-delete-assert
branch
from
db8ce6a to
d98ae96
Compare
|
Thank you. Is this a 2.3 PR then? |
|
Oh nevermind, just read you commend at the bug report. |
|
I'm on holiday and can't test it now. |
Close #11769