Skip to content

Commit

Permalink
Refactor FXIOS-8107 [v121.2] Disable recently visited section and loa…
Browse files Browse the repository at this point in the history 
…ding of the history highlights (backport #18060) (#18069)

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

* Refactor FXIOS-8107 [v121.2] Disable recently visited section and loading of the history highlights

* lint fix

* lint fixes

* test fixes

* test fixes v2

* test fixes v3

* test fixes v4

* Disabled full test

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 597220a)

# Conflicts:
#	firefox-ios/Tests/XCUITests/HomePageSettingsUITest.swift

* Fixed merge conflicts

* Bracket fix

* lint fix

---------

Co-authored-by: Nishant Bhasin <[email protected]>
  • Loading branch information
@mergify @nbhasin2
mergify[bot] and nbhasin2 authored Jan 10, 2024
1 parent 87727ea commit 19f1f1b
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 54 deletions.
8 changes: 3 additions & 5 deletions BrowserKit/Sources/Common/Extensions/URLExtension.swift
View file
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
8 changes: 6 additions & 2 deletions Client/Frontend/Home/HistoryHighlights/HistoryHighlightsDataAdaptor.swift
View file
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
2 changes: 2 additions & 0 deletions firefox-ios/Tests/ClientTests/Coordinators/ShortcutRouteTests.swift
View file
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
View file
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
1 change: 1 addition & 0 deletions ...Tests/ClientTests/Frontend/Home/HistoryHighlights/HistoryHighlightsDataAdaptorTests.swift
View file
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
2 changes: 1 addition & 1 deletion ...s/Tests/ClientTests/Frontend/Home/HistoryHighlights/HistoryHighlightsViewModelTests.swift
View file
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
2 changes: 2 additions & 0 deletions firefox-ios/Tests/ClientTests/Messaging/GleanPlumbMessageManagerTests.swift
View file
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
View file
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
View file
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
View file
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
View file
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

1 comment on commit 19f1f1b

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

InterpreterError at template.tasks[0].extra[0].treeherder[1].symbol: unknown context value cron

Please sign in to comment.