wordpress-mobile / wordpress-mobile/GutenbergKit

Error view is never added as a child view controller

Open Beginner friendly
#668 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

[Type] Bug [Type] Code Quality iOS
Dominant language
JavaScript
Stars
29
Forks
6
Avg merge
1d 9h
Merged PRs (30d)
41

Description

displayError calls didMove(toParent:) on a UIHostingController it never added as a child, so UIKit view-controller containment is never actually established.

Detail

ios/Sources/GutenbergKit/Sources/EditorViewController.swift:1040-1042:

self.errorViewController = UIHostingController(rootView: AnyView(view))
self.displayAndCenterView(errorViewController!.view)
self.errorViewController?.didMove(toParent: self)

addChild(_:) is what establishes the parent-child relationship; didMove(toParent:) is only the notification you send after it. Without the addChild, the hosting controller has no parent, so it is outside the containment hierarchy and receives no appearance or trait-collection callbacks.

The teardown half is missing too: hideError() (:1046) only does errorViewController?.view.removeFromSuperview() — no willMove(toParent: nil), no removeFromParent(), and it does not nil out errorViewController. It also has zero callers anywhere in ios/, which is its own problem (see #667).

How it bites

Anything the error view needs to react to stops working: Dynamic Type changes, light/dark mode switches, rotation, and size-class changes are never propagated to the hosting controller. Cosmetic today, because the content is a static ContentUnavailableView — but it is a trap for anyone who makes that view interactive or adds a retry button to it, which is exactly what #667 proposes.

There is also a force-unwrap on the line between (errorViewController!.view) that a plain let would avoid.

Suggested fix

let controller = UIHostingController(rootView: AnyView(view))
addChild(controller)
displayAndCenterView(controller.view)
controller.didMove(toParent: self)
self.errorViewController = controller

and the mirror in hideError():

errorViewController?.willMove(toParent: nil)
errorViewController?.view.removeFromSuperview()
errorViewController?.removeFromParent()
errorViewController = nil

Found while reviewing #651. Pre-existing; that PR does not touch this path.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read ios/Sources/GutenbergKit/Sources/EditorViewController.swift around displayError() and hideError(). Verify the error hosting controller is correctly contained and fully removed on teardown, including clearing errorViewController; done means both lifecycle paths establish and release UIKit containment without the force-unwrap.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
mobile
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.