wordpress-mobile / wordpress-mobile/GutenbergKit
Error view is never added as a child view controller
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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