From 0a5290d5a680014557c362b81fb87b896f105a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Mon, 14 Jan 2019 14:00:55 +0100 Subject: [PATCH] [apps/code] Fix MenuController::willExitResponderChain It called textFieldDidAbortEditing, which called setFirstResponder, which could clash with another setFirstResponder higher in the call tree. Scenario: build with an update popup, create a new script, edit its name with an unvalid name and press the Power key while editing. When powering on the device again, the first responder is not the popup even though it is displayed, and pressing OK does not dismiss it. --- apps/code/menu_controller.cpp | 65 +++++++++++++++++++---------------- apps/code/menu_controller.h | 5 ++- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/apps/code/menu_controller.cpp b/apps/code/menu_controller.cpp index 1c8e3823d..e7e04a8d4 100644 --- a/apps/code/menu_controller.cpp +++ b/apps/code/menu_controller.cpp @@ -49,7 +49,7 @@ void MenuController::willExitResponderChain(Responder * nextFirstResponder) { TextField * tf = static_cast(m_selectableTableView.selectedCell())->textField(); if (tf->isEditing()) { tf->setEditing(false, false); - textFieldDidAbortEditing(tf); + privateTextFieldDidAbortEditing(tf, false); } } } @@ -346,34 +346,6 @@ bool MenuController::textFieldDidFinishEditing(TextField * textField, const char return false; } -bool MenuController::textFieldDidAbortEditing(TextField * textField) { - Script script = m_scriptStore->scriptAtIndex(m_selectableTableView.selectedRow()); - const char * scriptName = script.fullName(); - if (strlen(scriptName) <= 1 + strlen(ScriptStore::k_scriptExtension)) { - // The previous text was an empty name. Use a numbered default script name. - char numberedDefaultName[Script::k_defaultScriptNameMaxSize]; - bool foundDefaultName = Script::DefaultName(numberedDefaultName, Script::k_defaultScriptNameMaxSize); - if (!foundDefaultName) { - // If we did not find a default name, delete the script - deleteScript(script); - return true; - } - Script::ErrorStatus error = script.setBaseNameWithExtension(numberedDefaultName, ScriptStore::k_scriptExtension); - scriptName = m_scriptStore->scriptAtIndex(m_selectableTableView.selectedRow()).fullName(); - /* Because we use the numbered default name, the name should not be - * already taken. Plus, the script could be added only if the storage has - * enough available space to add a script named 'script99.py' */ - (void) error; // Silence the "variable unused" warning if assertions are not enabled - assert(error == Script::ErrorStatus::None); - updateAddScriptRowDisplay(); - } - textField->setText(scriptName); - m_selectableTableView.selectCellAtLocation(m_selectableTableView.selectedColumn(), m_selectableTableView.selectedRow()); - app()->setFirstResponder(&m_selectableTableView); - static_cast(const_cast(app()->container()))->setShiftAlphaStatus(Ion::Events::ShiftAlphaStatus::Default); - return true; -} - bool MenuController::textFieldDidHandleEvent(TextField * textField, bool returnValue, bool textSizeDidChange) { int scriptExtensionLength = 1 + strlen(ScriptStore::k_scriptExtension); if (textField->isEditing() && textField->cursorLocation() > textField->draftTextLength() - scriptExtensionLength) { @@ -411,4 +383,39 @@ void MenuController::updateAddScriptRowDisplay() { m_selectableTableView.reloadData(); } +bool MenuController::privateTextFieldDidAbortEditing(TextField * textField, bool menuControllerStaysInResponderChain) { + /* If menuControllerStaysInResponderChain is false, we do not want to use + * methods that might call setFirstResponder, because we might be in the + * middle of another setFirstResponder call. */ + Script script = m_scriptStore->scriptAtIndex(m_selectableTableView.selectedRow()); + const char * scriptName = script.fullName(); + if (strlen(scriptName) <= 1 + strlen(ScriptStore::k_scriptExtension)) { + // The previous text was an empty name. Use a numbered default script name. + char numberedDefaultName[Script::k_defaultScriptNameMaxSize]; + bool foundDefaultName = Script::DefaultName(numberedDefaultName, Script::k_defaultScriptNameMaxSize); + if (!foundDefaultName) { + // If we did not find a default name, delete the script + deleteScript(script); + return true; + } + Script::ErrorStatus error = script.setBaseNameWithExtension(numberedDefaultName, ScriptStore::k_scriptExtension); + scriptName = m_scriptStore->scriptAtIndex(m_selectableTableView.selectedRow()).fullName(); + /* Because we use the numbered default name, the name should not be + * already taken. Plus, the script could be added only if the storage has + * enough available space to add a script named 'script99.py' */ + (void) error; // Silence the "variable unused" warning if assertions are not enabled + assert(error == Script::ErrorStatus::None); + if (menuControllerStaysInResponderChain) { + updateAddScriptRowDisplay(); + } + } + textField->setText(scriptName); + if (menuControllerStaysInResponderChain) { + m_selectableTableView.selectCellAtLocation(m_selectableTableView.selectedColumn(), m_selectableTableView.selectedRow()); + app()->setFirstResponder(&m_selectableTableView); + } + static_cast(const_cast(app()->container()))->setShiftAlphaStatus(Ion::Events::ShiftAlphaStatus::Default); + return true; +} + } diff --git a/apps/code/menu_controller.h b/apps/code/menu_controller.h index b4383549f..7b94014f4 100644 --- a/apps/code/menu_controller.h +++ b/apps/code/menu_controller.h @@ -52,7 +52,9 @@ public: bool textFieldShouldFinishEditing(TextField * textField, Ion::Events::Event event) override; bool textFieldDidReceiveEvent(TextField * textField, Ion::Events::Event event) override; bool textFieldDidFinishEditing(TextField * textField, const char * text, Ion::Events::Event event) override; - bool textFieldDidAbortEditing(TextField * textField) override; + bool textFieldDidAbortEditing(TextField * textField) override { + return privateTextFieldDidAbortEditing(textField, true); + } bool textFieldDidHandleEvent(TextField * textField, bool returnValue, bool textSizeDidChange) override; /* ButtonRowDelegate */ @@ -74,6 +76,7 @@ private: void editScriptAtIndex(int scriptIndex); void numberedDefaultScriptName(char * buffer); void updateAddScriptRowDisplay(); + bool privateTextFieldDidAbortEditing(TextField * textField, bool menuControllerStaysInResponderChain); ScriptStore * m_scriptStore; ScriptNameCell m_scriptCells[k_maxNumberOfDisplayableScriptCells]; EvenOddCellWithEllipsis m_scriptParameterCells[k_maxNumberOfDisplayableScriptCells];