From 4648263dc439a25a5e576deab686cf5476e25536 Mon Sep 17 00:00:00 2001 From: Shiki Date: Thu, 3 Jun 2021 15:11:20 -0600 Subject: [PATCH 01/10] Remove clearing of siteURLField This is causing an issue where when there's an XML-RPC error displayed, the text field will no longer be disabled when the user tries to submit another site URL. This was first created in cf6b66f6. As far as I can understand, there doesn't seem to be a critical need to keep this. The reference will still be pointed to the _current text field_ if the cell is recreated. --- .../Site Address/SiteAddressViewController.swift | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index 821a5d241..a4430b195 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -183,21 +183,6 @@ extension SiteAddressViewController: UITableViewDataSource { } } -// MARK: - UITableViewDelegate conformance -extension SiteAddressViewController: UITableViewDelegate { - /// After the site address textfield cell is done displaying, remove the textfield reference. - /// - func tableView(_ tableView: UITableView, didEndDisplaying cell: UITableViewCell, forRowAt indexPath: IndexPath) { - guard let row = rows[safe: indexPath.row] else { - return - } - - if row == .siteAddress { - siteURLField = nil - } - } -} - // MARK: - Keyboard Notifications extension SiteAddressViewController: NUXKeyboardResponder { @objc func handleKeyboardWillShow(_ notification: Foundation.Notification) { From 450d1e5f043faa3ef64087bdbd92cdf6271212ef Mon Sep 17 00:00:00 2001 From: Shiki Date: Thu, 3 Jun 2021 15:15:23 -0600 Subject: [PATCH 02/10] Retain the text (URL) that the user entered When an XML-RPC error is displayed, the URL that the user entered gets cleared. This retains the text to match with our other error handling UIs like the fancy dialog which does not clear the text field. --- .../View Related/Site Address/SiteAddressViewController.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index a4430b195..15b807ffc 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -289,6 +289,7 @@ private extension SiteAddressViewController { // Save a reference to the first textField so it can becomeFirstResponder. siteURLField = cell.textField cell.textField.delegate = self + cell.textField.text = loginFields.siteAddress cell.onChangeSelectionHandler = { [weak self] textfield in self?.loginFields.siteAddress = textfield.nonNilTrimmedText() self?.configureSubmitButton(animating: false) From 7e84acfe74b2f6a652dc1af774d2115b179f169d Mon Sep 17 00:00:00 2001 From: Shiki Date: Thu, 3 Jun 2021 16:13:23 -0600 Subject: [PATCH 03/10] SiteAddressVC: Add viewIsLoading state --- .../View Related/Site Address/SiteAddressViewController.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index 15b807ffc..d500dd616 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -19,6 +19,8 @@ final class SiteAddressViewController: LoginViewController { private var errorMessage: String? private var shouldChangeVoiceOverFocus: Bool = false + private var viewIsLoading: Bool = false + // MARK: - Actions @IBAction func handleContinueButtonTapped(_ sender: NUXButton) { tracker.track(click: .submit) @@ -120,6 +122,8 @@ final class SiteAddressViewController: LoginViewController { /// - Parameter loading: True if the form should be configured to a "loading" state. /// override func configureViewLoading(_ loading: Bool) { + viewIsLoading = loading + siteURLField?.isEnabled = !loading configureSubmitButton(animating: loading) From c8d95bc743f82925feb5a149ab5e906626f8d954 Mon Sep 17 00:00:00 2001 From: Shiki Date: Thu, 3 Jun 2021 16:14:22 -0600 Subject: [PATCH 04/10] SiteAddressVC: Add configureSubmitButton helper This function should be used instead of the one with the `animating: loading` argument to avoid incorrectly disabling the loading state of the button. --- .../Site Address/SiteAddressViewController.swift | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index d500dd616..ff7e9130a 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -38,7 +38,7 @@ final class SiteAddressViewController: LoginViewController { localizePrimaryButton() registerTableViewCells() loadRows() - configureSubmitButton(animating: false) + configureSubmitButton() configureForAccessibility() } @@ -46,7 +46,7 @@ final class SiteAddressViewController: LoginViewController { super.viewWillAppear(animated) siteURLField?.text = loginFields.siteAddress - configureSubmitButton(animating: false) + configureSubmitButton() } override func viewDidAppear(_ animated: Bool) { @@ -84,6 +84,10 @@ final class SiteAddressViewController: LoginViewController { return WordPressAuthenticator.shared.unifiedStyle?.statusBarStyle ?? WordPressAuthenticator.shared.style.statusBarStyle } + private func configureSubmitButton() { + configureSubmitButton(animating: viewIsLoading) + } + /// Configures the appearance and state of the submit button. /// override func configureSubmitButton(animating: Bool) { @@ -126,7 +130,7 @@ final class SiteAddressViewController: LoginViewController { siteURLField?.isEnabled = !loading - configureSubmitButton(animating: loading) + configureSubmitButton() navigationItem.hidesBackButton = loading } @@ -296,7 +300,7 @@ private extension SiteAddressViewController { cell.textField.text = loginFields.siteAddress cell.onChangeSelectionHandler = { [weak self] textfield in self?.loginFields.siteAddress = textfield.nonNilTrimmedText() - self?.configureSubmitButton(animating: false) + self?.configureSubmitButton() } SigninEditingState.signinEditingStateActive = true From 52dfad96dd40ab63fff49bc565eeb13c2132f916 Mon Sep 17 00:00:00 2001 From: Shiki Date: Tue, 8 Jun 2021 10:51:26 -0600 Subject: [PATCH 05/10] Add comments to SiteAddressVC --- .../Site Address/SiteAddressViewController.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index ff7e9130a..21d99c143 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -19,6 +19,13 @@ final class SiteAddressViewController: LoginViewController { private var errorMessage: String? private var shouldChangeVoiceOverFocus: Bool = false + /// A state variable that is `true` if network calls are currently happening and so the + /// view should be showing a loading indicator. + /// + /// This should only be modified within `configureViewLoading(_ loading:)`. + /// + /// This state is mainly used in `configureSubmitButton()` to determine whether the button + /// should show an activity indicator. private var viewIsLoading: Bool = false // MARK: - Actions @@ -84,6 +91,10 @@ final class SiteAddressViewController: LoginViewController { return WordPressAuthenticator.shared.unifiedStyle?.statusBarStyle ?? WordPressAuthenticator.shared.style.statusBarStyle } + /// Configures the appearance and state of the submit button. + /// + /// Use this instead of the overridden `configureSubmitButton(animating:)` since this uses the + /// _current_ `viewIsLoading` state. private func configureSubmitButton() { configureSubmitButton(animating: viewIsLoading) } From 592608365daae953bb29920657426445e27bde48 Mon Sep 17 00:00:00 2001 From: Shiki Date: Tue, 8 Jun 2021 10:59:45 -0600 Subject: [PATCH 06/10] Increment version to 1.38.0-beta.3 --- WordPressAuthenticator.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPressAuthenticator.podspec b/WordPressAuthenticator.podspec index 0971ae8a4..8f2c94242 100644 --- a/WordPressAuthenticator.podspec +++ b/WordPressAuthenticator.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "WordPressAuthenticator" - s.version = "1.38.0-beta.2" + s.version = "1.38.0-beta.3" s.summary = "WordPressAuthenticator implements an easy and elegant way to authenticate your WordPress Apps." s.description = <<-DESC From d5f68c34fa825835d43533954fd393242a85e896 Mon Sep 17 00:00:00 2001 From: Shiki Date: Tue, 8 Jun 2021 13:54:16 -0600 Subject: [PATCH 07/10] SiteAddressVC: Use the verified site url This fixes this bug: 1. Log in with a site address that has a blocked XML-RPC. For example, https://do.wpmt.co/ddos. 2. After the error is displayed, log in with a valid self-hosted site address. 3. Continue entering the credentials to log in. 4. You will not be able to. This also does not work with WooCommerce. The WC app will spit out a "...is connected to a different account" error. --- .../SiteAddressViewController.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index 21d99c143..b30e4e851 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -494,6 +494,25 @@ private extension SiteAddressViewController { } func presentNextControllerIfPossible(siteInfo: WordPressComSiteInfo?) { + + // Ensure that we're using the verified URL before passing the `loginFields` to the next + // view controller. + // + // In some scenarios, the text field change callback in `configureTextField()` gets executed + // right after we validated and modified `loginFields.siteAddress` in `validateForm()`. And + // this causes the value of `loginFields.siteAddress` to be reset to what the user entered. + // + // Using the user-entered `loginFields.siteAddress` causes problems when we try to log + // the user in. For example, validating their self-hosted site credentials somehow + // fails. Especially if they only entered the domain (no url scheme). + // + // This routine fixes that problem. We'll use what we already validated from + // `fetchSiteInfo()`. + // + if let verifiedSiteAddress = siteInfo?.url { + loginFields.siteAddress = verifiedSiteAddress + } + guard siteInfo?.isWPCom == false else { showGetStarted() return From 45d02a709a9c010131a320767c227d2fe072c146 Mon Sep 17 00:00:00 2001 From: Shiki Date: Tue, 8 Jun 2021 15:52:59 -0600 Subject: [PATCH 08/10] Add test that baseSiteURL() keeps the "http" --- .../Authenticator/WordPressAuthenticatorTests.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/WordPressAuthenticatorTests/Authenticator/WordPressAuthenticatorTests.swift b/WordPressAuthenticatorTests/Authenticator/WordPressAuthenticatorTests.swift index 25b09b04d..8c6c7d37f 100644 --- a/WordPressAuthenticatorTests/Authenticator/WordPressAuthenticatorTests.swift +++ b/WordPressAuthenticatorTests/Authenticator/WordPressAuthenticatorTests.swift @@ -49,6 +49,12 @@ class WordPressAuthenticatorTests: XCTestCase { XCTAssert(url == punycode) } + func testBaseSiteURLKeepsHTTPSchemeForNonWPSites() { + let url = "http://selfhostedsite.com" + let correctedURL = WordPressAuthenticator.baseSiteURL(string: url) + XCTAssertEqual(correctedURL, url) + } + // MARK: WordPressAuthenticator Notification Tests func testDispatchesSupportPushNotificationReceived() { let authenticator = WordpressAuthenticatorProvider.getWordpressAuthenticator() From aa020aa62cc50f6bc24e51d0fb6f64bcf871d79b Mon Sep 17 00:00:00 2001 From: Shiki Date: Tue, 8 Jun 2021 16:56:29 -0600 Subject: [PATCH 09/10] Add more explanation --- .../Site Address/SiteAddressViewController.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index b30e4e851..7d41415b9 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -503,8 +503,9 @@ private extension SiteAddressViewController { // this causes the value of `loginFields.siteAddress` to be reset to what the user entered. // // Using the user-entered `loginFields.siteAddress` causes problems when we try to log - // the user in. For example, validating their self-hosted site credentials somehow - // fails. Especially if they only entered the domain (no url scheme). + // the user in especially if they just use a domain. For example, validating their + // self-hosted site credentials fails because the + // `WordPressOrgXMLRPCValidator.guessXMLRPCURLForSite` expects a complete site URL. // // This routine fixes that problem. We'll use what we already validated from // `fetchSiteInfo()`. From d6447e73cb8357b6c7d5adb85eca61687b3dfb76 Mon Sep 17 00:00:00 2001 From: Shiki Date: Wed, 9 Jun 2021 19:32:53 -0600 Subject: [PATCH 10/10] Remove unnecessary print() --- .../View Related/Site Address/SiteAddressViewController.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift index 7d41415b9..354ea033d 100644 --- a/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift +++ b/WordPressAuthenticator/Unified Auth/View Related/Site Address/SiteAddressViewController.swift @@ -467,7 +467,6 @@ private extension SiteAddressViewController { } func fetchSiteInfo() { - print("🔴 SAVC > fetchSiteInfo") let baseSiteUrl = WordPressAuthenticator.baseSiteURL(string: loginFields.siteAddress) let service = WordPressComBlogService()