Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FXIOS-8107 [v121.2] Disable recently visited section and loading of the history highlights (backport #18060) #18069

Merged
merged 4 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions BrowserKit/Sources/Common/Extensions/URLExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import Foundation
extension URL {
/// Temporary init that will be removed with the update to XCode 15 where this URL API is available
public init?(string: String, invalidCharacters: Bool) {
if #available(iOS 17, *) {
self.init(string: string, encodingInvalidCharacters: invalidCharacters)
} else {
self.init(string: string)
}
// FXIOS-8107: Removed 'encodingInvalidCharacters' init for
// compatibility reasons that is available for iOS 17+ only
self.init(string: string)
}

/// Returns a shorter displayable string for a domain
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ protocol HistoryHighlightsDelegate: AnyObject {
func didLoadNewData()
}

class HistoryHighlightsDataAdaptorImplementation: HistoryHighlightsDataAdaptor {
class HistoryHighlightsDataAdaptorImplementation: HistoryHighlightsDataAdaptor, FeatureFlaggable {
private var historyItems = [HighlightItem]()
private var historyManager: HistoryHighlightsManagerProtocol
private var profile: Profile
Expand Down Expand Up @@ -85,7 +85,11 @@ extension HistoryHighlightsDataAdaptorImplementation: Notifiable {
switch notification.name {
case .HistoryUpdated,
.RustPlacesOpened:
loadHistory()
// FXIOS-8107: Disabling loadHistory as it is causing the app to slow down on frequent calls
// "recent-explorations" in homescreenFeature.yaml has been set to false for all builds
if featureFlags.isFeatureEnabled(.historyHighlights, checking: .buildOnly) {
loadHistory()
}
default:
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ final class ShortcutRouteTests: XCTestCase {
options: [.switchToNormalMode]))
}

// FXIOS-8107: Disabled test as history highlights has been disabled to fix app hangs / slowness
// Reloads for notification
func testOpenLastBookmarkShortcutWithInvalidUrl() {
let subject = createSubject()
let userInfo = [QuickActionInfos.tabURLKey: "not a url" as NSSecureCoding]
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/Tests/ClientTests/FeatureFlagManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class FeatureFlagManagerTests: XCTestCase, FeatureFlaggable {
// Technically, at this stage, these should be the same.
XCTAssertTrue(featureFlags.isFeatureEnabled(.bottomSearchBar, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.bottomSearchBar, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.historyHighlights, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.historyHighlights, checking: .userOnly))
XCTAssertFalse(featureFlags.isFeatureEnabled(.historyHighlights, checking: .buildOnly))
XCTAssertFalse(featureFlags.isFeatureEnabled(.historyHighlights, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.historyGroups, checking: .buildOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.historyGroups, checking: .userOnly))
XCTAssertTrue(featureFlags.isFeatureEnabled(.inactiveTabs, checking: .buildOnly))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class HistoryHighlightsDataAdaptorTests: XCTestCase {
XCTAssertEqual(delegate.didLoadNewDataCallCount, 1)
}

// FXIOS-8107: Disabled test as history highlights has been disabled to fix app hangs / slowness
// Reloads for notification
func testReloadDataOnNotification() {
historyManager.callGetHighlightsDataCompletion(result: [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class HistoryHighlightsViewModelTests: XCTestCase {
subject.didLoadNewData()

XCTAssertEqual(subject.getItemDetailsAt(index: 0)?.displayTitle, "mozilla")
XCTAssertEqual(delegate.reloadViewCallCount, 1)
XCTAssertEqual(delegate.reloadViewCallCount, 0)
}

func testLoadNewDataIsNotEnabled() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ class GleanPlumbMessageManagerTests: XCTestCase {
testEventMetricRecordingSuccess(metric: GleanMetrics.Messaging.clicked)
}

// FXIOS-8107: Disabled test as history highlights has been disabled to fix app hangs / slowness
// Reloads for notification
func testManagerOnMessagePressed_withMalformedURL() {
let message = createMessage(messageId: messageId, action: "http://www.google.com?q=א")
subject.onMessagePressed(message)
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Tests/ExperimentIntegrationTests.xctestplan
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"FxScreenGraphTests",
"HistoryTests",
"HomeButtonTests",
"HomePageSettingsUITests",
"HomePageSettingsUITests\/testRecentlyVisited()",
"IntegrationTests",
"IpadOnlyTestCase",
"IphoneOnlyTestCase",
Expand Down
3 changes: 3 additions & 0 deletions firefox-ios/Tests/UnitTest.xctestplan
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,12 @@
{
"skippedTests" : [
"ETPCoverSheetTests",
"GleanPlumbMessageManagerTests\/testManagerOnMessagePressed_withMalformedURL()",
"HistoryHighlightsDataAdaptorTests\/testReloadDataOnNotification()",
"IntroViewControllerTests\/testBasicSetupReturnsExpectedItems()",
"RustSyncManagerTests\/testGetEnginesAndKeysWithNoKey()",
"RustSyncManagerTests\/testUpdateEnginePrefs_bookmarksEnabled()",
"ShortcutRouteTests\/testOpenLastBookmarkShortcutWithInvalidUrl()",
"TabManagerTests\/testDeleteSelectedTab()",
"TabManagerTests\/testPrivatePreference_togglePBMDeletesPrivate()",
"TestFavicons\/testFaviconFetcherParse()",
Expand Down
84 changes: 44 additions & 40 deletions firefox-ios/Tests/XCUITests/HomePageSettingsUITest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ class HomePageSettingsUITests: BaseTestCase {
XCTAssertEqual("1", jumpBackIn as? String)
let recentlySaved = app.tables.cells.switches["Recently Saved"].value
XCTAssertEqual("1", recentlySaved as? String)
let recentlyVisited = app.tables.cells.switches["Recently Visited"].value
XCTAssertEqual("1", recentlyVisited as? String)
// FXIOS-8107: Commented out as history highlights has been disabled to fix app hangs / slowness
// Reloads for notification
// let recentlyVisited = app.tables.cells.switches["Recently Visited"].value
// XCTAssertEqual("1", recentlyVisited as? String)
let sponsoredStories = app.tables.cells.switches["Thought-Provoking Stories, Articles powered by Pocket"].value
XCTAssertEqual("1", sponsoredStories as? String)

Expand Down Expand Up @@ -265,43 +267,45 @@ class HomePageSettingsUITests: BaseTestCase {

// https://testrail.stage.mozaws.net/index.php?/cases/view/2306923
// Smoketest
func testRecentlyVisited() {
navigator.openURL(websiteUrl1)
waitUntilPageLoad()
navigator.performAction(Action.GoToHomePage)
mozWaitForElementToExist(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel])
navigator.goto(HomeSettings)
navigator.performAction(Action.ToggleRecentlyVisited)

// On iPad we have the homepage button always present,
// on iPhone we have the search button instead when we're on a new tab page
if !iPad() {
navigator.performAction(Action.ClickSearchButton)
} else {
navigator.performAction(Action.GoToHomePage)
}

XCTAssertFalse(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel].exists)
if !iPad() {
mozWaitForElementToExist(app.buttons["urlBar-cancel"], timeout: 3)
navigator.performAction(Action.CloseURLBarOpen)
}
navigator.nowAt(NewTabScreen)
navigator.goto(HomeSettings)
navigator.performAction(Action.ToggleRecentlyVisited)
navigator.nowAt(HomeSettings)
navigator.performAction(Action.OpenNewTabFromTabTray)
XCTAssert(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel].exists)

// Disabled due to https://github.com/mozilla-mobile/firefox-ios/issues/11271
// navigator.openURL("mozilla ")
// navigator.openURL(websiteUrl2)
// FXIOS-8107: Disabled test as history highlights has been disabled to fix app hangs / slowness
// Reloads for notification
// func testRecentlyVisited() {
// navigator.openURL(websiteUrl1)
// waitUntilPageLoad()
// navigator.performAction(Action.GoToHomePage)
// XCTAssert(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].exists)
// app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].staticTexts["Mozilla , Pages: 2"].press(forDuration: 1.5)
// selectOptionFromContextMenu(option: "Remove")
// XCTAssertFalse(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].exists)
}
// mozWaitForElementToExist(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel])
// navigator.goto(HomeSettings)
// navigator.performAction(Action.ToggleRecentlyVisited)
//
// // On iPad we have the homepage button always present,
// // on iPhone we have the search button instead when we're on a new tab page
// if !iPad() {
// navigator.performAction(Action.ClickSearchButton)
// } else {
// navigator.performAction(Action.GoToHomePage)
// }
//
// XCTAssertFalse(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel].exists)
// if !iPad() {
// mozWaitForElementToExist(app.buttons["urlBar-cancel"], timeout: 3)
// navigator.performAction(Action.CloseURLBarOpen)
// }
// navigator.nowAt(NewTabScreen)
// navigator.goto(HomeSettings)
// navigator.performAction(Action.ToggleRecentlyVisited)
// navigator.nowAt(HomeSettings)
// navigator.performAction(Action.OpenNewTabFromTabTray)
// XCTAssert(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts[urlMozillaLabel].exists)
//
//// Disabled due to https://github.com/mozilla-mobile/firefox-ios/issues/11271
//// navigator.openURL("mozilla ")
//// navigator.openURL(websiteUrl2)
//// navigator.performAction(Action.GoToHomePage)
//// XCTAssert(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].exists)
//// app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].staticTexts["Mozilla , Pages: 2"].press(forDuration: 1.5)
//// selectOptionFromContextMenu(option: "Remove")
//// XCTAssertFalse(app.scrollViews.cells[AccessibilityIdentifiers.FirefoxHomepage.HistoryHighlights.itemCell].staticTexts["Mozilla , Pages: 2"].exists)
// }

// https://testrail.stage.mozaws.net/index.php?/cases/view/2306871
// Smoketest
Expand All @@ -310,7 +314,6 @@ class HomePageSettingsUITests: BaseTestCase {
mozWaitForElementToExist(app.collectionViews["FxCollectionView"], timeout: TIMEOUT)
app.collectionViews["FxCollectionView"].swipeUp()
app.collectionViews["FxCollectionView"].swipeUp()
app.collectionViews["FxCollectionView"].swipeUp()
mozWaitForElementToExist(app.cells.otherElements.buttons[AccessibilityIdentifiers.FirefoxHomepage.MoreButtons.customizeHomePage], timeout: TIMEOUT)
}
app.cells.otherElements.buttons[AccessibilityIdentifiers.FirefoxHomepage.MoreButtons.customizeHomePage].tap()
Expand All @@ -322,7 +325,8 @@ class HomePageSettingsUITests: BaseTestCase {
// Commented due to experimental features
// XCTAssertEqual(app.cells.switches[AccessibilityIdentifiers.Settings.Homepage.CustomizeFirefox.jumpBackIn].value as! String, "1")
// XCTAssertEqual(app.cells.switches[AccessibilityIdentifiers.Settings.Homepage.CustomizeFirefox.recentlySaved].value as! String, "1")
XCTAssertEqual(app.cells.switches[AccessibilityIdentifiers.Settings.Homepage.CustomizeFirefox.recentVisited].value as! String, "1")
// FXIOS-8107: Commented out as history highlights has been disabled to fix app hangs / slowness
// XCTAssertEqual(app.cells.switches[AccessibilityIdentifiers.Settings.Homepage.CustomizeFirefox.recentVisited].value as! String, "1")
XCTAssertEqual(app.cells.switches["Thought-Provoking Stories, Articles powered by Pocket"].value as! String, "1")
}
}
6 changes: 3 additions & 3 deletions nimbus-features/homescreenFeature.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ features:
default:
{
"jump-back-in": true,
"recent-explorations": true,
"recent-explorations": false,
}
pocket-sponsored-stories:
description: >
Expand All @@ -23,15 +23,15 @@ features:
value: {
"sections-enabled": {
"jump-back-in": true,
"recent-explorations": true,
"recent-explorations": false,
},
"pocket-sponsored-stories": true
}
- channel: beta
value: {
"sections-enabled": {
"jump-back-in": true,
"recent-explorations": true,
"recent-explorations": false,
},
"pocket-sponsored-stories": false
}
Expand Down