From 4ddf839474588adb1822cf429c70472dd6ed943e Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 11 Nov 2016 00:26:57 -0700 Subject: [PATCH] The big history-fixer-upper - History code was refactored - one step forward with regard to https://github.com/brave/browser-laptop/issues/3502 - session helper created for aboutHistory (tests added) - history specific methods broke out into historyUtil (tests added) - history stores it's data in AppStore.about.history (similar structure to new tab code) - about:history is only updated when something changed - # calls to render on about:history is drastically reduced (should fix https://github.com/brave/browser-laptop/issues/5382; please reopen if not) - about:brave text update Implements followup feedback for https://github.com/brave/browser-laptop/pull/5436#issuecomment-259144537 Auditors: @cezaraugusto, @darkdh Test Plan: 1. Launch Brave and go to about:history 2. With history open, open a new tab and visit a site you have never been to before (hint: just append #1234567 to the end of URL) 3. Switch back to history tab and confirm entry shows 4. Open another new tab; in the URL bar, type an entry for a site you HAVE been to before (which shows in the autocomplete) 5. Instead of hitting enter, pick the URL from the autocomplete drop down (use arrow keys and press enter, OR click) 6. Visit about:history again and confirm this entry shows 7. On about:history, click some of the entries. They should show their selection status in a faster manner than before Advanced test notes: - After exiting Brave, you can view the session file to confirm the details used in about:history and about:brave are removed and NOT stored to disk ----- Remove unnecessary/broken/useless context menu items for about pages Fixes https://github.com/brave/browser-laptop/issues/4812 ----- Update icon for about pages to fa-list (except for about:newtab) Fixes https://github.com/brave/browser-laptop/issues/5497 Auditors: @bradleyrichter Test Plan: 1. Launch Brave and visit about:about 2. Open each of the links and confirm it's the `fa-list` icon --- app/common/lib/historyUtil.js | 27 ++++++-- app/common/state/aboutHistoryState.js | 21 +++++++ app/renderer/components/urlBarIcon.js | 10 ++- app/sessionStore.js | 16 +++-- docs/appActions.md | 6 ++ docs/state.md | 9 +++ js/about/history.js | 63 +++++++------------ js/actions/appActions.js | 9 +++ js/components/frame.js | 9 +-- js/components/main.js | 31 ++++++--- js/constants/appConstants.js | 3 +- js/contextMenus.js | 54 +++++++++------- js/stores/appStore.js | 6 ++ test/components/navigationBarTest.js | 27 +++++--- .../common/state/aboutHistoryStateTest.js | 38 +++++++++++ test/unit/lib/historyUtilTest.js | 34 ++++++---- 16 files changed, 258 insertions(+), 105 deletions(-) create mode 100644 app/common/state/aboutHistoryState.js create mode 100644 test/unit/common/state/aboutHistoryStateTest.js diff --git a/app/common/lib/historyUtil.js b/app/common/lib/historyUtil.js index df232cc35e4..108590b073f 100644 --- a/app/common/lib/historyUtil.js +++ b/app/common/lib/historyUtil.js @@ -2,7 +2,26 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ 'use strict' + const Immutable = require('immutable') +const {makeImmutable} = require('../state/immutableUtil') +const siteUtil = require('../../../js/state/siteUtil') +const aboutHistoryMaxEntries = 500 + +module.exports.maxEntries = aboutHistoryMaxEntries + +const sortTimeDescending = (left, right) => { + if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1 + if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1 + return 0 +} + +module.exports.getHistory = (sites) => { + sites = makeImmutable(sites) || new Immutable.List() + return sites.filter((site) => siteUtil.isHistoryEntry(site)) + .sort(sortTimeDescending) + .slice(0, aboutHistoryMaxEntries) +} const getDayString = (entry, locale) => { const lastAccessedTime = entry.get('lastAccessedTime') @@ -42,15 +61,11 @@ module.exports.groupEntriesByDay = (history, locale) => { * Format is expected to be array containing one array per day. */ module.exports.totalEntries = (entriesByDay) => { + entriesByDay = makeImmutable(entriesByDay) || new Immutable.List() + let result = new Immutable.List() entriesByDay.forEach((entry) => { result = result.push(entry.get('entries')) }) return result } - -module.exports.sortTimeDescending = (left, right) => { - if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) return 1 - if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) return -1 - return 0 -} diff --git a/app/common/state/aboutHistoryState.js b/app/common/state/aboutHistoryState.js new file mode 100644 index 00000000000..6f9048993e6 --- /dev/null +++ b/app/common/state/aboutHistoryState.js @@ -0,0 +1,21 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const {makeImmutable} = require('./immutableUtil') +const historyUtil = require('../lib/historyUtil') + +const aboutHistoryState = { + getHistory: (state) => { + state = makeImmutable(state) + return state.getIn(['about', 'history']) + }, + setHistory: (state) => { + state = makeImmutable(state) + state = state.setIn(['about', 'history', 'entries'], + historyUtil.getHistory(state.get('sites'))) + return state.setIn(['about', 'history', 'updatedStamp'], new Date().getTime()) + } +} + +module.exports = aboutHistoryState diff --git a/app/renderer/components/urlBarIcon.js b/app/renderer/components/urlBarIcon.js index 693f5a8ed26..e8871434cf9 100644 --- a/app/renderer/components/urlBarIcon.js +++ b/app/renderer/components/urlBarIcon.js @@ -47,10 +47,15 @@ class UrlBarIcon extends ImmutableComponent { const defaultToSearch = (!this.isSecure && !this.isInsecure && !showSearch) && !this.props.titleMode && - this.props.loading === false + this.props.loading === false && + !this.isAboutPage return showSearch || defaultToSearch } + get isAboutPage () { + return isSourceAboutUrl(this.props.location) && + this.props.location !== 'about:newtab' + } get iconClasses () { if (this.props.activateSearchEngine) { return cx({urlbarIcon: true}) @@ -62,7 +67,8 @@ class UrlBarIcon extends ImmutableComponent { // NOTE: EV style not approved yet; see discussion at https://github.com/brave/browser-laptop/issues/791 'fa-lock': this.isSecure, 'fa-exclamation-triangle': this.isInsecure, - 'fa-search': this.isSearch + 'fa-search': this.isSearch, + 'fa-list': this.isAboutPage }) } get iconStyles () { diff --git a/app/sessionStore.js b/app/sessionStore.js index fa086127e2b..46c4915439b 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -327,7 +327,10 @@ module.exports.cleanAppData = (data, isShutdown) => { }) } - delete data.versionInformation + if (data.about) { + delete data.about.brave + delete data.about.history + } } /** @@ -435,18 +438,21 @@ module.exports.loadAppState = () => { // version information (shown on about:brave) const os = require('os') const versionInformation = [ - {name: 'brave', version: app.getVersion()}, - {name: 'muon', version: process.versions['atom-shell']}, + {name: 'Brave', version: app.getVersion()}, + {name: 'Muon', version: process.versions['atom-shell']}, {name: 'libchromiumcontent', version: process.versions['chrome']}, {name: 'V8', version: process.versions.v8}, {name: 'Node.js', version: process.versions.node}, - {name: 'channel', version: Channel.channel()}, + {name: 'Update Channel', version: Channel.channel()}, {name: 'os.platform', version: os.platform()}, {name: 'os.release', version: os.release()}, {name: 'os.arch', version: os.arch()} // TODO(bsclifton): read the latest commit hash from a file, etc. ] - data.versionInformation = versionInformation + data.about = data.about || {} + data.about.brave = { + versionInformation: versionInformation + } } catch (e) { // TODO: Session state is corrupted, maybe we should backup this // corrupted value for people to report into support. diff --git a/docs/appActions.md b/docs/appActions.md index a067405cfb7..b76ea3b97ff 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -494,6 +494,12 @@ Dispatch a message to indicate default browser check is complete +### populateHistory() + +Notify the AppStore to provide default history values. + + + * * * diff --git a/docs/state.md b/docs/state.md index 12edcc84af0..ebca6386415 100644 --- a/docs/state.md +++ b/docs/state.md @@ -220,6 +220,15 @@ AppStore } }, about: { + brave: { + versionInformation: [{ + name: string, + version: string + }] // used on about:brave. not persisted (removed on save) + }, + history: { + entries: [object] // used on about:history. not persisted (removed on save) + }, newtab: { gridLayoutSize: string, // 'small', 'medium', 'large' sites: [string], // List of sites to be used on gridLayout diff --git a/js/about/history.js b/js/about/history.js index b54894c0b2d..d6af81d9586 100644 --- a/js/about/history.js +++ b/js/about/history.js @@ -13,7 +13,6 @@ const aboutActions = require('./aboutActions') const getSetting = require('../settings').getSetting const SortableTable = require('../components/sortableTable') const Button = require('../components/button') -const siteUtil = require('../state/siteUtil') const {makeImmutable} = require('../../app/common/state/immutableUtil') const historyUtil = require('../../app/common/lib/historyUtil') @@ -56,13 +55,6 @@ class HistoryTimeCell extends ImmutableComponent { } class HistoryDay extends ImmutableComponent { - constructor () { - super() - this.clearSelection = this.clearSelection.bind(this) - } - clearSelection () { - this.refs.historyTable.clearSelection() - } navigate (entry) { entry = makeImmutable(entry) aboutActions.newFrame({ @@ -74,7 +66,6 @@ class HistoryDay extends ImmutableComponent { return
{this.props.date}
[ @@ -101,36 +92,23 @@ class HistoryDay extends ImmutableComponent { } } -class GroupedHistoryList extends ImmutableComponent { - constructor () { - super() - this.entriesByDay = [] - this.clearSelection = this.clearSelection.bind(this) - } - clearSelection () { - this.entriesByDay.forEach((day) => day.clearSelection()) - } +class GroupedHistoryList extends React.Component { render () { const defaultLanguage = this.props.languageCodes.find((lang) => lang.includes(navigator.language)) || 'en-US' const userLanguage = getSetting(settings.LANGUAGE, this.props.settings) || defaultLanguage const entriesByDay = historyUtil.groupEntriesByDay(this.props.history, userLanguage) const totalEntries = historyUtil.totalEntries(entriesByDay) let index = 0 - this.entriesByDay = [] return { - entriesByDay.map((groupedEntry) => { - const day = + - this.entriesByDay.push(this.refs[('historyDay' + index++)]) - return day - }) + />) } } @@ -149,10 +127,22 @@ class AboutHistory extends React.Component { search: '', settings: Immutable.Map(), languageCodes: Immutable.Map(), - selection: Immutable.Set() + selection: Immutable.Set(), + updatedStamp: undefined } ipc.on(messages.HISTORY_UPDATED, (e, detail) => { - this.setState({ history: Immutable.fromJS(detail && detail.history || {}) }) + const aboutHistory = Immutable.fromJS(detail || {}) + const updatedStamp = aboutHistory.get('updatedStamp') + // Only update if the data has changed. + if (typeof updatedStamp === 'number' && + typeof this.state.updatedStamp === 'number' && + updatedStamp === this.state.updatedStamp) { + return + } + this.setState({ + history: aboutHistory.get('entries') || new Immutable.List(), + updatedStamp: updatedStamp + }) }) ipc.on(messages.SETTINGS_UPDATED, (e, settings) => { this.setState({ settings: Immutable.fromJS(settings || {}) }) @@ -180,7 +170,6 @@ class AboutHistory extends React.Component { } targetElement = targetElement.parentNode } - // Click was not a child element of sortableTable; clear selection this.clearSelection() } @@ -190,16 +179,13 @@ class AboutHistory extends React.Component { return title.match(new RegExp(searchTerm, 'gi')) }) } - get historyDescendingOrder () { - return this.state.history.filter((site) => siteUtil.isHistoryEntry(site)) - .sort(historyUtil.sortTimeDescending) - .slice(0, 500) - } clearBrowsingDataNow () { aboutActions.clearBrowsingDataNow({browserHistory: true}) } clearSelection () { - this.refs.historyList.clearSelection() + this.setState({ + selection: new Immutable.Set() + }) } componentDidMount () { this.refs.historySearch.focus() @@ -223,13 +209,12 @@ class AboutHistory extends React.Component {
diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 0121a2d2826..ff0f89f05d8 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -579,6 +579,15 @@ const appActions = { AppDispatcher.dispatch({ actionType: AppConstants.APP_DEFAULT_BROWSER_CHECK_COMPLETE }) + }, + + /** + * Notify the AppStore to provide default history values. + */ + populateHistory: function () { + AppDispatcher.dispatch({ + actionType: AppConstants.APP_POPULATE_HISTORY + }) } } diff --git a/js/components/frame.js b/js/components/frame.js index 73d5a1cc77a..4c83e67737f 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -96,9 +96,10 @@ class Frame extends ImmutableComponent { bookmarkFolders: this.props.bookmarkFolders.toJS() }) } else if (location === 'about:history') { - this.webview.send(messages.HISTORY_UPDATED, { - history: this.props.history.toJS() - }) + const aboutHistoryState = this.props.history && this.props.history.toJS + ? this.props.history.toJS() + : {} + this.webview.send(messages.HISTORY_UPDATED, aboutHistoryState) this.webview.send(messages.SETTINGS_UPDATED, this.props.settings ? this.props.settings.toJS() : null) } else if (location === 'about:extensions') { this.webview.send(messages.EXTENSIONS_UPDATED, { @@ -171,7 +172,7 @@ class Frame extends ImmutableComponent { this.webview.send(messages.AUTOFILL_CREDIT_CARDS_UPDATED, list) } } else if (location === 'about:brave') { - const versionInformation = appStoreRenderer.state.get('versionInformation') + const versionInformation = appStoreRenderer.state.getIn(['about', 'brave', 'versionInformation']) if (versionInformation && versionInformation.toJS) { this.webview.send(messages.VERSION_INFORMATION_UPDATED, versionInformation.toJS()) } diff --git a/js/components/main.js b/js/components/main.js index 9f46a215e68..13a3a60ab1b 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -59,6 +59,7 @@ const {bookmarksToolbarMode} = require('../../app/common/constants/settingsEnums // State handling const basicAuthState = require('../../app/common/state/basicAuthState') const extensionState = require('../../app/common/state/extensionState') +const aboutHistoryState = require('../../app/common/state/aboutHistoryState') const FrameStateUtil = require('../state/frameStateUtil') const siteUtil = require('../state/siteUtil') const searchProviders = require('../data/searchProviders') @@ -837,6 +838,17 @@ class Main extends ImmutableComponent { } } + bindHistory (frame) { + if (frame.get('location') === 'about:history') { + const history = aboutHistoryState.getHistory(this.props.appState) + if (history) { + return history + } + appActions.populateHistory() + } + return null + } + render () { const comparatorByKeyAsc = (a, b) => a.get('key') > b.get('key') ? 1 : b.get('key') > a.get('key') ? -1 : 0 @@ -887,6 +899,8 @@ class Main extends ImmutableComponent { activeFrame && !activeFrame.getIn(['security', 'loginRequiredDetail']) && !customTitlebar.menubarSelectedIndex + const appStateSites = this.props.appState.get('sites') + return
{ this.navBar = node }} navbar={activeFrame && activeFrame.get('navbar')} frames={this.props.windowState.get('frames')} - sites={this.props.appState.get('sites')} + sites={appStateSites} activeFrameKey={activeFrame && activeFrame.get('key') || undefined} location={activeFrame && activeFrame.get('location') || ''} title={activeFrame && activeFrame.get('title') || ''} @@ -1080,7 +1094,8 @@ class Main extends ImmutableComponent { } { this.props.windowState.get('bookmarkDetail') && !this.props.windowState.getIn(['bookmarkDetail', 'isBookmarkHanger']) - ? : null } @@ -1158,7 +1173,7 @@ class Main extends ImmutableComponent { tabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'tabPageIndex'])} previewTabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'previewTabPageIndex'])} tabs={this.props.windowState.get('tabs')} - sites={this.props.appState.get('sites')} + sites={appStateSites} key='tab-bar' activeFrameKey={activeFrame && activeFrame.get('key') || undefined} onMenu={this.onHamburgerMenu} @@ -1193,13 +1208,11 @@ class Main extends ImmutableComponent { ? this.props.appState.get('settings') || emptyMap : null} bookmarks={frame.get('location') === 'about:bookmarks' - ? this.props.appState.get('sites') + ? appStateSites .filter((site) => site.get('tags') .includes(siteTags.BOOKMARK)) || emptyMap : null} - history={frame.get('location') === 'about:history' - ? this.props.appState.get('sites') || emptyMap - : null} + history={this.bindHistory(frame)} extensions={frame.get('location') === 'about:extensions' ? this.props.appState.get('extensions') || emptyMap : null} @@ -1208,7 +1221,7 @@ class Main extends ImmutableComponent { : null} downloads={this.props.appState.get('downloads') || emptyMap} bookmarkFolders={frame.get('location') === 'about:bookmarks' - ? this.props.appState.get('sites') + ? appStateSites .filter((site) => site.get('tags') .includes(siteTags.BOOKMARK_FOLDER)) || emptyMap : null} diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 9ee5bc7c697..9ef853f4d76 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -63,7 +63,8 @@ const AppConstants = { APP_UPDATE_ADBLOCK_CUSTOM_RULES: _, APP_SUBMIT_FEEDBACK: _, APP_DEFAULT_BROWSER_UPDATED: _, - APP_DEFAULT_BROWSER_CHECK_COMPLETE: _ + APP_DEFAULT_BROWSER_CHECK_COMPLETE: _, + APP_POPULATE_HISTORY: _ } module.exports = mapValuesByKeys(AppConstants) diff --git a/js/contextMenus.js b/js/contextMenus.js index c802a554512..6a2cb927a4e 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -28,7 +28,7 @@ const locale = require('../js/l10n') const {getSetting} = require('./settings') const settings = require('./constants/settings') const textUtils = require('./lib/text') -const {isIntermediateAboutPage, isUrl} = require('./lib/appUrlUtil') +const {isIntermediateAboutPage, isUrl, aboutUrls} = require('./lib/appUrlUtil') const {getBase64FromImageUrl} = require('./lib/imageUtil') const urlParse = require('url').parse const eventUtil = require('./lib/eventUtil') @@ -899,6 +899,7 @@ function mainTemplateInit (nodeProps, frame) { const isAudio = nodeProps.mediaType === 'audio' const isInputField = nodeProps.isEditable || nodeProps.inputFieldType !== 'none' const isTextSelected = nodeProps.selectionText.length > 0 + const isAboutPage = aboutUrls.has(frame.get('location')) if (isLink) { template.push(openInNewTabMenuItem(nodeProps.linkURL, frame.get('isPrivate'), frame.get('partitionNumber'), frame.get('key')), @@ -1052,35 +1053,44 @@ function mainTemplateInit (nodeProps, frame) { } }, CommonMenu.separatorMenuItem, - addBookmarkMenuItem('bookmarkPage', siteUtil.getDetailFromFrame(frame, siteTags.BOOKMARK), false), - { + addBookmarkMenuItem('bookmarkPage', siteUtil.getDetailFromFrame(frame, siteTags.BOOKMARK), false)) + + if (!isAboutPage) { + template.push({ label: locale.translation('savePageAs'), accelerator: 'CmdOrCtrl+S', click: function (item, focusedWindow) { CommonMenu.sendToFocusedWindow(focusedWindow, [messages.SHORTCUT_ACTIVE_FRAME_SAVE]) } - }, { - label: locale.translation('find'), - accelerator: 'CmdOrCtrl+F', - click: function (item, focusedWindow) { - focusedWindow.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_SHOW_FINDBAR) - } - }, { + }) + } + + template.push({ + label: locale.translation('find'), + accelerator: 'CmdOrCtrl+F', + click: function (item, focusedWindow) { + focusedWindow.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_SHOW_FINDBAR) + } + }) + + if (!isAboutPage) { + template.push({ label: locale.translation('print'), accelerator: 'CmdOrCtrl+P', click: function (item, focusedWindow) { focusedWindow.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_PRINT) } - } - // CommonMenu.separatorMenuItem - // TODO: bravery menu goes here - ) + }) + } + + // CommonMenu.separatorMenuItem + // TODO: bravery menu goes here } template.push(CommonMenu.separatorMenuItem) } - if (!isLink && !isImage) { + if (!isLink && !isImage && !isAboutPage) { template.push({ label: locale.translation('viewPageSource'), accelerator: 'CmdOrCtrl+Alt+U', @@ -1093,12 +1103,14 @@ function mainTemplateInit (nodeProps, frame) { } } - template.push({ - label: locale.translation('inspectElement'), - click: (item, focusedWindow) => { - webviewActions.inspectElement(nodeProps.x, nodeProps.y) - } - }) + if (!isAboutPage) { + template.push({ + label: locale.translation('inspectElement'), + click: (item, focusedWindow) => { + webviewActions.inspectElement(nodeProps.x, nodeProps.y) + } + }) + } const extensionContextMenus = extensionState.getContextMenusProperties(appStore.state) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 664b57d6124..a3b66642337 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -39,6 +39,7 @@ const basicAuthState = require('../../app/common/state/basicAuthState') const extensionState = require('../../app/common/state/extensionState') const tabState = require('../../app/common/state/tabState') const aboutNewTabState = require('../../app/common/state/aboutNewTabState') +const aboutHistoryState = require('../../app/common/state/aboutHistoryState') const isDarwin = process.platform === 'darwin' const isWindows = process.platform === 'win32' @@ -435,6 +436,9 @@ const handleAppAction = (action) => { appState = aboutNewTabState.setSites(appState, action) } break + case AppConstants.APP_POPULATE_HISTORY: + appState = aboutHistoryState.setHistory(appState, action) + break case AppConstants.APP_ADD_SITE: const oldSiteSize = appState.get('sites').size if (action.siteDetail.constructor === Immutable.List) { @@ -452,10 +456,12 @@ const handleAppAction = (action) => { filterOutNonRecents() } appState = aboutNewTabState.setSites(appState, action) + appState = aboutHistoryState.setHistory(appState, action) break case AppConstants.APP_REMOVE_SITE: appState = appState.set('sites', siteUtil.removeSite(appState.get('sites'), action.siteDetail, action.tag)) appState = aboutNewTabState.setSites(appState, action) + appState = aboutHistoryState.setHistory(appState, action) break case AppConstants.APP_MOVE_SITE: appState = appState.set('sites', siteUtil.moveSite(appState.get('sites'), action.sourceDetail, action.destinationDetail, action.prepend, action.destinationIsParent, false)) diff --git a/test/components/navigationBarTest.js b/test/components/navigationBarTest.js index 06e92e7d130..34f34545431 100644 --- a/test/components/navigationBarTest.js +++ b/test/components/navigationBarTest.js @@ -873,14 +873,14 @@ describe('navigationBar tests', function () { }) }) - describe('with blank url input value', function () { - Brave.beforeAll(this) + describe('with about page url input values', function () { + Brave.beforeEach(this) - before(function * () { + beforeEach(function * () { yield setup(this.app.client) yield this.app.client.waitForExist(urlInput) }) - it('hides about:newtab', function * () { + it('hides "about:newtab" inside the URL bar', function * () { yield this.app.client .tabByUrl(this.newTabUrl) .windowByUrl(Brave.browserWindowUrl) @@ -888,7 +888,7 @@ describe('navigationBar tests', function () { return this.getValue(urlInput).then((val) => val === '') }) }) - it('shows about:blank', function * () { + it('shows "about:blank" in the URL bar', function * () { yield this.app.client .keys('about:blank') .keys(Brave.keys.ENTER) @@ -896,8 +896,21 @@ describe('navigationBar tests', function () { return this.getValue(urlInput).then((val) => val === 'about:blank') }) }) - it('has the search icon', function * () { - yield this.app.client.waitForExist('.urlbarIcon.fa-search') + it('shows the search icon in URL bar for "about:newtab"', function * () { + yield this.app.client + .tabByUrl(this.newTabUrl) + .windowByUrl(Brave.browserWindowUrl) + .waitForExist('.urlbarIcon.fa-search') + }) + it('shows the list icon in URL bar for other about pages', function * () { + yield this.app.client + .windowByUrl(Brave.browserWindowUrl) + .keys('about:about') + .keys(Brave.keys.ENTER) + .waitUntil(function () { + return this.getValue(urlInput).then((val) => val === 'about:about') + }) + .waitForExist('.urlbarIcon.fa-list') }) }) diff --git a/test/unit/common/state/aboutHistoryStateTest.js b/test/unit/common/state/aboutHistoryStateTest.js new file mode 100644 index 00000000000..eb1f13f13fe --- /dev/null +++ b/test/unit/common/state/aboutHistoryStateTest.js @@ -0,0 +1,38 @@ +/* global describe, it */ +const aboutHistoryState = require('../../../../app/common/state/aboutHistoryState') +const Immutable = require('immutable') +const assert = require('assert') + +const defaultAppState = Immutable.fromJS({ + about: { + history: { + entries: [], + updatedStamp: undefined + } + } +}) + +const arbitraryTimeInThePast = 1450000000000 + +const assertTimeUpdated = (state) => { + const updatedStamp = state.getIn(['about', 'history', 'updatedStamp']) + assert.equal(typeof updatedStamp === 'number' && updatedStamp > arbitraryTimeInThePast, true) +} + +describe('aboutHistoryState', function () { + describe('getHistory', function () { + it('reads the history from the state', function () { + const fakeHistoryEntries = [1, 2, 3] + const state = defaultAppState.setIn(['about', 'history', 'entries'], fakeHistoryEntries) + const history = aboutHistoryState.getHistory(state) + assert.deepEqual(state.getIn(['about', 'history']).toJS(), history.toJS()) + }) + }) + + describe('setHistory', function () { + it('updates the `updatedStamp` value on success', function () { + const state = aboutHistoryState.setHistory(defaultAppState) + assertTimeUpdated(state) + }) + }) +}) diff --git a/test/unit/lib/historyUtilTest.js b/test/unit/lib/historyUtilTest.js index d0580813d7c..bcd6f484faf 100644 --- a/test/unit/lib/historyUtilTest.js +++ b/test/unit/lib/historyUtilTest.js @@ -35,6 +35,29 @@ describe('historyUtil', function () { }]) const historyMultipleDays = historyDayThree.push(historyDayTwo, historyDayOne) + describe('getHistory', function () { + it('returns the result as an Immutable.List', function () { + const result = historyUtil.getHistory(historyMultipleDays) + assert.equal(Immutable.List.isList(result), true) + }) + it('sorts the items by date/time DESC', function () { + const result = historyUtil.getHistory(historyMultipleDays) + const expectedResult = historyDayThree.toJS().reverse() + expectedResult.push(historyDayTwo.toJS()) + expectedResult.push(historyDayOne.toJS()) + assert.deepEqual(result.toJS(), expectedResult) + }) + it('only returns `historyUtil.maxEntries` results', function () { + let tooManyEntries = new Immutable.List().push(historyDayOne) + for (let i = 0; i < historyUtil.maxEntries; i++) { + tooManyEntries = tooManyEntries.push(historyDayOne) + } + assert.equal(tooManyEntries.size, (historyUtil.maxEntries + 1)) + const result = historyUtil.getHistory(tooManyEntries) + assert.equal(result.size, historyUtil.maxEntries) + }) + }) + describe('groupEntriesByDay', function () { it('returns the result as an Immutable.List', function () { const result = historyUtil.groupEntriesByDay(historyDayThree) @@ -78,15 +101,4 @@ describe('historyUtil', function () { assert.deepEqual(result2.toJS(), expectedResult) }) }) - - describe('sortTimeDescending', function () { - it('sorts the items by date/time DESC', function () { - const result = historyMultipleDays.sort(historyUtil.sortTimeDescending) - const expectedResult = historyDayThree.toJS().reverse() - expectedResult.push(historyDayTwo.toJS()) - expectedResult.push(historyDayOne.toJS()) - - assert.deepEqual(result.toJS(), expectedResult) - }) - }) })