From b5b8d51b0d1a6acd608a7099076aa3c8ac287566 Mon Sep 17 00:00:00 2001 From: Alexander Vakrilov Date: Wed, 20 Jun 2018 16:10:03 +0300 Subject: [PATCH] fix(modal): parent page invalid hierarchy handling [extended] (#5966) * fix(modal): parent page invalid hierarchy handling * refactor(modals): Refactor safe guard in show/hide modal --- .../ui/core/bindable/bindable.d.ts | 12 +++++- tns-core-modules/ui/core/bindable/bindable.ts | 12 +++++- tns-core-modules/ui/core/view/view.d.ts | 6 ++- tns-core-modules/ui/core/view/view.ios.ts | 37 ++++++++++++------- tns-core-modules/ui/dialogs/dialogs.ios.ts | 3 +- 5 files changed, 51 insertions(+), 19 deletions(-) diff --git a/tns-core-modules/ui/core/bindable/bindable.d.ts b/tns-core-modules/ui/core/bindable/bindable.d.ts index b626fbca6..4010401ed 100644 --- a/tns-core-modules/ui/core/bindable/bindable.d.ts +++ b/tns-core-modules/ui/core/bindable/bindable.d.ts @@ -4,11 +4,19 @@ import { ViewBase } from "../view-base"; import { Observable, WrappedValue, PropertyChangeData, EventData } from "../../../data/observable"; -import { isEnabled as traceEnabled, write as traceWrite, categories as traceCategories, notifyEvent as traceNotifyEvent, messageType as traceMessageType, isCategorySet } from "../../../trace"; +import { + isEnabled as traceEnabled, + write as traceWrite, + error as traceError, + categories as traceCategories, + notifyEvent as traceNotifyEvent, + messageType as traceMessageType, + isCategorySet } from "../../../trace"; export { Observable, WrappedValue, PropertyChangeData, EventData, - traceEnabled, traceWrite, traceCategories, traceNotifyEvent, traceMessageType, isCategorySet + traceEnabled, traceWrite, traceError, traceCategories, traceNotifyEvent, + traceMessageType, isCategorySet }; /** diff --git a/tns-core-modules/ui/core/bindable/bindable.ts b/tns-core-modules/ui/core/bindable/bindable.ts index 636e3d2ed..00df4723f 100644 --- a/tns-core-modules/ui/core/bindable/bindable.ts +++ b/tns-core-modules/ui/core/bindable/bindable.ts @@ -6,7 +6,15 @@ import { Observable, WrappedValue, PropertyChangeData, EventData } from "../../. import { addWeakEventListener, removeWeakEventListener } from "../weak-event-listener"; import { bindingConstants, parentsRegex } from "../../builder/binding-builder"; import { escapeRegexSymbols } from "../../../utils/utils"; -import { isEnabled as traceEnabled, write as traceWrite, categories as traceCategories, notifyEvent as traceNotifyEvent, isCategorySet, messageType as traceMessageType } from "../../../trace"; +import { + isEnabled as traceEnabled, + write as traceWrite, + error as traceError, + categories as traceCategories, + notifyEvent as traceNotifyEvent, + isCategorySet, + messageType as traceMessageType +} from "../../../trace"; import * as types from "../../../utils/types"; import * as applicationCommon from "../../../application/application-common"; @@ -14,7 +22,7 @@ import * as polymerExpressions from "../../../js-libs/polymer-expressions"; export { Observable, WrappedValue, PropertyChangeData, EventData, - traceEnabled, traceWrite, traceCategories, traceNotifyEvent, traceMessageType, isCategorySet + traceEnabled, traceWrite, traceError, traceCategories, traceNotifyEvent, traceMessageType, isCategorySet }; const contextKey = "context"; diff --git a/tns-core-modules/ui/core/view/view.d.ts b/tns-core-modules/ui/core/view/view.d.ts index f040ce2e6..8fccb6425 100644 --- a/tns-core-modules/ui/core/view/view.d.ts +++ b/tns-core-modules/ui/core/view/view.d.ts @@ -775,7 +775,11 @@ export const isEnabledProperty: Property; export const isUserInteractionEnabledProperty: Property; export namespace ios { - export function getParentWithViewController(parent: View): View + /** + * Returns a view with viewController or undefined if no such found along the view's parent chain. + * @param view The view form which to start the search. + */ + export function getParentWithViewController(view: View): View export function isContentScrollable(controller: any /* UIViewController */, owner: View): boolean export function updateAutoAdjustScrollInsets(controller: any /* UIViewController */, owner: View): void export function updateConstraints(controller: any /* UIViewController */, owner: View): void; diff --git a/tns-core-modules/ui/core/view/view.ios.ts b/tns-core-modules/ui/core/view/view.ios.ts index 72b0830a1..a50543d97 100644 --- a/tns-core-modules/ui/core/view/view.ios.ts +++ b/tns-core-modules/ui/core/view/view.ios.ts @@ -4,7 +4,7 @@ import { ViewBase } from "../view-base"; import { ViewCommon, layout, isEnabledProperty, originXProperty, originYProperty, automationTextProperty, isUserInteractionEnabledProperty, - traceEnabled, traceWrite, traceCategories, traceMessageType + traceEnabled, traceWrite, traceCategories, traceError, traceMessageType } from "./view-common"; import { ios as iosBackground, Background } from "../../styling/background"; @@ -309,7 +309,19 @@ export class View extends ViewCommon { } protected _showNativeModalView(parent: View, context: any, closeCallback: Function, fullscreen?: boolean, animated?: boolean, stretched?: boolean) { - let parentWithController = ios.getParentWithViewController(parent); + const parentWithController = ios.getParentWithViewController(parent); + if (!parentWithController) { + traceWrite(`Could not find parent with viewController for ${parent} while showing modal view.`, + traceCategories.ViewHierarchy, traceMessageType.error) + return; + } + + const parentController = parentWithController.viewController; + if (!parentController.view || !parentController.view.window) { + traceWrite("Parent page is not part of the window hierarchy. Close the current modal page before showing another one!", + traceCategories.ViewHierarchy, traceMessageType.error); + return; + } super._showNativeModalView(parentWithController, context, closeCallback, fullscreen, stretched); let controller = this.viewController; @@ -326,11 +338,6 @@ export class View extends ViewCommon { this._setupAsRootView({}); - const parentController = parentWithController.viewController; - if (!parentController.view.window) { - throw new Error("Parent page is not part of the window hierarchy. Close the current modal page before showing another one!"); - } - if (fullscreen) { controller.modalPresentationStyle = UIModalPresentationStyle.FullScreen; } else { @@ -346,7 +353,8 @@ export class View extends ViewCommon { parentController.presentViewControllerAnimatedCompletion(controller, animated, null); const transitionCoordinator = iosUtils.getter(parentController, parentController.transitionCoordinator); if (transitionCoordinator) { - UIViewControllerTransitionCoordinator.prototype.animateAlongsideTransitionCompletion.call(transitionCoordinator, null, () => this._raiseShownModallyEvent()); + UIViewControllerTransitionCoordinator.prototype.animateAlongsideTransitionCompletion + .call(transitionCoordinator, null, () => this._raiseShownModallyEvent()); } else { // Apparently iOS 9+ stops all transitions and animations upon application suspend and transitionCoordinator becomes null here in this case. // Since we are not waiting for any transition to complete, i.e. transitionCoordinator is null, we can directly raise our shownModally event. @@ -356,6 +364,11 @@ export class View extends ViewCommon { } protected _hideNativeModalView(parent: View) { + if (!parent || !parent.viewController) { + traceError("Trying to hide modal view but no parent with viewController specified.") + return; + } + const parentController = parent.viewController; const animated = (this.viewController).animated; @@ -582,14 +595,12 @@ export class CustomLayoutView extends View { } export namespace ios { - export function getParentWithViewController(parent: View): View { - let view = parent; - let controller = view.viewController; - while (!controller) { + export function getParentWithViewController(view: View): View { + while (view && !view.viewController) { view = view.parent as View; - controller = view.viewController; } + // Note: Might return undefined if no parent with viewController is found return view; } diff --git a/tns-core-modules/ui/dialogs/dialogs.ios.ts b/tns-core-modules/ui/dialogs/dialogs.ios.ts index cb4da410c..4cb1afaa9 100644 --- a/tns-core-modules/ui/dialogs/dialogs.ios.ts +++ b/tns-core-modules/ui/dialogs/dialogs.ios.ts @@ -209,7 +209,8 @@ function showUIAlertController(alertController: UIAlertController) { if (view.ios instanceof UIViewController) { viewController = view.ios; } else { - viewController = iosView.getParentWithViewController(view).viewController; + const parentWithController = iosView.getParentWithViewController(view); + viewController = parentWithController ? parentWithController.viewController : undefined; } }