Skip to content

Commit

Permalink
Add a notice about other forms of browsing history to the History WebUI.
Browse files Browse the repository at this point in the history
Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC1cuEjM/

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332

Review URL: https://codereview.chromium.org/1838333004

Cr-Commit-Position: refs/heads/master@{#384903}
  • Loading branch information
msramek authored and Commit bot committed Apr 4, 2016
1 parent 63a392e commit 5872007
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 22 deletions.
16 changes: 14 additions & 2 deletions chrome/browser/resources/history/history.css
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,23 @@ html[dir='rtl'] #notification-bar {
float: left;
}

#notification-bar.alone {
#top-container.overflow #notification-bar {
float: left;
margin-top: 12px;
}

html[dir='rtl'] #notification-bar.alone {
html[dir='rtl'] #top-container.overflow #notification-bar {
float: right;
}

#notification-bar span {
display: block;
}

#notification-bar span + span {
margin: 1em 0;
}

#filter-controls,
#top-container,
#results-display,
Expand All @@ -73,6 +81,10 @@ html[dir='rtl'] #editing-controls {
float: right;
}

#top-container.overflow #editing-controls {
float: none;
}

#editing-controls button:first-of-type {
-webkit-margin-start: 0;
}
Expand Down
60 changes: 46 additions & 14 deletions chrome/browser/resources/history/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -644,12 +644,6 @@ HistoryModel.prototype.addResults = function(info, results) {
lastDay = thisDay;
}

if (loadTimeData.getBoolean('isUserSignedIn')) {
var message = loadTimeData.getString(
info.hasSyncedResults ? 'hasSyncedResults' : 'noSyncedResults');
this.view_.showNotification(message);
}

this.updateSearch_();
};

Expand Down Expand Up @@ -1166,6 +1160,31 @@ HistoryView.prototype.showNotification = function(innerHTML, isWarning) {
this.positionNotificationBar();
};

/**
* Shows a notification about whether there are any synced results, and whether
* there are other forms of browsing history on the server.
* @param {boolean} hasSyncedResults Whether there are synced results.
* @param {boolean} includeOtherFormsOfBrowsingHistory Whether to include
* a sentence about the existence of other forms of browsing history.
*/
HistoryView.prototype.showWebHistoryNotification = function(
hasSyncedResults, includeOtherFormsOfBrowsingHistory) {
var message = '';

if (loadTimeData.getBoolean('isUserSignedIn')) {
message += '<span>' + loadTimeData.getString(
hasSyncedResults ? 'hasSyncedResults' : 'noSyncedResults') + '</span>';
}

if (includeOtherFormsOfBrowsingHistory) {
message += ' ' /* A whitespace to separate <span>s. */ + '<span>' +
loadTimeData.getString('otherFormsOfBrowsingHistory') + '</span>';
}

if (message)
this.showNotification(message);
};

/**
* @param {Visit} visit The visit about to be removed from this view.
*/
Expand Down Expand Up @@ -1274,15 +1293,15 @@ HistoryView.prototype.onEntryRemoved = function() {
*/
HistoryView.prototype.positionNotificationBar = function() {
var bar = $('notification-bar');
var container = $('top-container');

// If the bar does not fit beside the editing controls, put it into the
// overflow state.
if (bar.getBoundingClientRect().top >=
$('editing-controls').getBoundingClientRect().bottom) {
bar.classList.add('alone');
} else {
bar.classList.remove('alone');
}
// If the bar does not fit beside the editing controls, or if it contains
// more than one message, put it into the overflow state.
var shouldOverflow =
(bar.getBoundingClientRect().top >=
$('editing-controls').getBoundingClientRect().bottom) ||
bar.childElementCount > 1;
container.classList.toggle('overflow', shouldOverflow);
};

/**
Expand Down Expand Up @@ -2345,6 +2364,19 @@ function historyResult(info, results) {
historyModel.addResults(info, results);
}

/**
* Called by the history backend after receiving results and after discovering
* the existence of other forms of browsing history.
* @param {boolean} hasSyncedResults Whether there are synced results.
* @param {boolean} includeOtherFormsOfBrowsingHistory Whether to include
* a sentence about the existence of other forms of browsing history.
*/
function showNotification(
hasSyncedResults, includeOtherFormsOfBrowsingHistory) {
historyView.showWebHistoryNotification(
hasSyncedResults, includeOtherFormsOfBrowsingHistory);
}

/**
* Called by the history backend when history removal is successful.
*/
Expand Down
21 changes: 19 additions & 2 deletions chrome/browser/resources/history/history_mobile.css
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ html {
}

body {
background-color: rgba(0, 0, 0, .05);
color: rgb(76, 76, 76);
font-size: initial;
height: 100%;
Expand All @@ -32,6 +33,7 @@ body {
}

h1 {
color: rgb(34, 34, 34);
font-weight: bold;
margin-bottom: 12px;
}
Expand Down Expand Up @@ -87,9 +89,20 @@ html[dir='rtl'] #search-field {
background-position: right 16px center;
}

#notification-bar.alone {
#notification-bar {
color: rgb(138, 138, 138);
font-size: 14px;
line-height: 1.5;
}

#notification-bar span {
/* On desktop, notification spans are displayed as separate paragraphs.
On mobile, join them into a single paragraph. */
display: inline;
}

#top-container.overflow #notification-bar {
float: none;
font-size: 75%;
margin: 0;
padding-bottom: 0;
padding-top: 0;
Expand Down Expand Up @@ -123,6 +136,10 @@ button.remove-entry:active {
opacity: 1.0;
}

.entry {
background-color: white;
}

.entry-box {
margin-bottom: 0;
margin-top: 0;
Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/resources/md_history/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ function historyResult(info, results) {
});
}

/**
* Called by the history backend after receiving results and after discovering
* the existence of other forms of browsing history.
* @param {boolean} hasSyncedResults Whether there are synced results.
* @param {boolean} includeOtherFormsOfBrowsingHistory Whether to include
* a sentence about the existence of other forms of browsing history.
*/
function showNotification(
hasSyncedResults, includeOtherFormsOfBrowsingHistory) {
// TODO(msramek): Implement the joint notification about web history and other
// forms of browsing history for the MD history page.
}

/**
* Receives the synced history data. An empty list means that either there are
* no foreign sessions, or tab sync is disabled for this profile.
Expand Down
29 changes: 28 additions & 1 deletion chrome/browser/ui/webui/browsing_history_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/browser_sync/browser/profile_sync_service.h"
#include "components/browsing_data_ui/history_notice_utils.h"
#include "components/favicon/core/fallback_icon_service.h"
#include "components/favicon/core/fallback_url_util.h"
#include "components/favicon/core/large_icon_service.h"
Expand Down Expand Up @@ -310,6 +311,8 @@ bool BrowsingHistoryHandler::HistoryEntry::SortByTimeDescending(
BrowsingHistoryHandler::BrowsingHistoryHandler()
: has_pending_delete_request_(false),
history_service_observer_(this),
has_synced_results_(false),
has_other_forms_of_browsing_history_(false),
weak_factory_(this) {
}

Expand Down Expand Up @@ -388,6 +391,8 @@ void BrowsingHistoryHandler::QueryHistory(

query_results_.clear();
results_info_value_.Clear();
has_synced_results_ = false;
has_other_forms_of_browsing_history_ = false;

history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
Expand Down Expand Up @@ -418,6 +423,14 @@ void BrowsingHistoryHandler::QueryHistory(
web_history_timer_.Start(
FROM_HERE, base::TimeDelta::FromSeconds(kWebHistoryTimeoutSeconds),
this, &BrowsingHistoryHandler::WebHistoryTimeout);

// Test the existence of other forms of browsing history.
browsing_data_ui::ShouldShowNoticeAboutOtherFormsOfBrowsingHistory(
ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile),
web_history,
base::Bind(
&BrowsingHistoryHandler::OtherFormsOfBrowsingHistoryQueryComplete,
weak_factory_.GetWeakPtr()));
}
}

Expand Down Expand Up @@ -690,6 +703,10 @@ void BrowsingHistoryHandler::ReturnResultsToFrontEnd() {

web_ui()->CallJavascriptFunction(
"historyResult", results_info_value_, results_value);
web_ui()->CallJavascriptFunction(
"showNotification",
base::FundamentalValue(has_synced_results_),
base::FundamentalValue(has_other_forms_of_browsing_history_));
results_info_value_.Clear();
query_results_.clear();
web_history_query_results_.clear();
Expand Down Expand Up @@ -829,11 +846,21 @@ void BrowsingHistoryHandler::WebHistoryQueryComplete(
}
}
}
results_info_value_.SetBoolean("hasSyncedResults", results_value != NULL);
has_synced_results_ = results_value != nullptr;
results_info_value_.SetBoolean("hasSyncedResults", has_synced_results_);
if (!query_task_tracker_.HasTrackedTasks())
ReturnResultsToFrontEnd();
}

void BrowsingHistoryHandler::OtherFormsOfBrowsingHistoryQueryComplete(
bool found_other_forms_of_browsing_history) {
has_other_forms_of_browsing_history_ = found_other_forms_of_browsing_history;
web_ui()->CallJavascriptFunction(
"showNotification",
base::FundamentalValue(has_synced_results_),
base::FundamentalValue(has_other_forms_of_browsing_history_));
}

void BrowsingHistoryHandler::RemoveComplete() {
urls_to_be_deleted_.clear();

Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/ui/webui/browsing_history_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
history::WebHistoryService::Request* request,
const base::DictionaryValue* results_value);

// Callback telling us whether other forms of browsing history were found
// on the history server.
void OtherFormsOfBrowsingHistoryQueryComplete(
bool found_other_forms_of_browsing_history);

// Callback from the history system when visits were deleted.
void RemoveComplete();

Expand Down Expand Up @@ -218,6 +223,12 @@ class BrowsingHistoryHandler : public content::WebUIMessageHandler,
ScopedObserver<history::HistoryService, history::HistoryServiceObserver>
history_service_observer_;

// Whether the last call to Web History returned synced results.
bool has_synced_results_;

// Whether there are other forms of browsing history on the history server.
bool has_other_forms_of_browsing_history_;

base::WeakPtrFactory<BrowsingHistoryHandler> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(BrowsingHistoryHandler);
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/ui/webui/history_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ static const char kOtherDevicesJsFile[] = "other_devices.js";

namespace {

static const char kMyActivityUrl[] =
"https://history.google.com/history/?utm_source=chrome_h";

#if defined(OS_MACOSX)
const char kIncognitoModeShortcut[] = "("
"\xE2\x87\xA7" // Shift symbol (U+21E7 'UPWARDS WHITE ARROW').
Expand Down Expand Up @@ -128,6 +131,10 @@ content::WebUIDataSource* CreateHistoryUIHTMLSource(Profile* profile) {
source->AddLocalizedString("hasSyncedResults",
IDS_HISTORY_HAS_SYNCED_RESULTS);
source->AddLocalizedString("noSyncedResults", IDS_HISTORY_NO_SYNCED_RESULTS);
source->AddString("otherFormsOfBrowsingHistory",
l10n_util::GetStringFUTF16(
IDS_HISTORY_OTHER_FORMS_OF_HISTORY,
base::ASCIIToUTF16(kMyActivityUrl)));
source->AddLocalizedString("cancel", IDS_CANCEL);
source->AddLocalizedString("deleteConfirm",
IDS_HISTORY_DELETE_PRIOR_VISITS_CONFIRM_BUTTON);
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/webui/history_browsertest.js
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ HistoryWebUIRealBackendTest.prototype = {
// AX_TEXT_04: http://crbug.com/560914
this.accessibilityAuditConfig.ignoreSelectors(
'linkWithUnclearPurpose',
'#notification-bar > A');
'#notification-bar > SPAN > A');
},
};

Expand Down
7 changes: 5 additions & 2 deletions components/history_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ Psst! Incognito mode <ph name="SHORTCUT_KEY">$1<ex>(Ctrl+Shift+N)</ex></ph> may
Group domains
</message>
<message name="IDS_HISTORY_HAS_SYNCED_RESULTS" desc="The notification at the top of the history page indicating that it is showing visits synced from other devices.">
Showing history from your signed-in devices. <ph name="BEGIN_LINK">&lt;a href="https://support.google.com/chrome/?p=sync_history&amp;hl=[GRITLANGCODE]"&gt;</ph>Learn more<ph name="END_LINK">&lt;/a&gt;<ex>&lt;/a&gt;</ex></ph>
Showing history from your signed-in devices. <ph name="BEGIN_LINK">&lt;a href="https://support.google.com/chrome/?p=sync_history&amp;hl=[GRITLANGCODE]"&gt;</ph>Learn more<ph name="END_LINK">&lt;/a&gt;<ex>&lt;/a&gt;</ex></ph>.
</message>
<message name="IDS_HISTORY_OTHER_FORMS_OF_HISTORY" desc="The notification at the top of the history page indicating that deleting Chrome browsing history will not delete other forms of history stored at Google My Activity.">
Your Google Account may have other forms of browsing history at <ph name="BEGIN_LINK">&lt;a target="_blank" href="$1"&gt;</ph>history.google.com<ph name="END_LINK">&lt;/a&gt;</ph>.
</message>
<message name="IDS_HISTORY_IN_CONTENT_PACK" desc="Text that shows that an entry is in a content pack.">
In content pack
Expand All @@ -80,7 +83,7 @@ Psst! Incognito mode <ph name="SHORTCUT_KEY">$1<ex>(Ctrl+Shift+N)</ex></ph> may
No search results found.
</message>
<message name="IDS_HISTORY_NO_SYNCED_RESULTS" desc="The notification at the top of the history page indicating that it does not include visits from other devices.">
Showing history from this device. <ph name="BEGIN_LINK">&lt;a href="https://support.google.com/chrome/?p=sync_history&amp;hl=[GRITLANGCODE]"&gt;</ph>Learn more<ph name="END_LINK">&lt;/a&gt;<ex>&lt;/a&gt;</ex></ph>
Showing history from this device. <ph name="BEGIN_LINK">&lt;a href="https://support.google.com/chrome/?p=sync_history&amp;hl=[GRITLANGCODE]"&gt;</ph>Learn more<ph name="END_LINK">&lt;/a&gt;<ex>&lt;/a&gt;</ex></ph>.
</message>
<message name="IDS_HISTORY_NUMBER_VISITS" desc="Format string for the number of visits of a site.">
(<ph name="NUMBER_VISITS">$1<ex>3</ex></ph>)
Expand Down

0 comments on commit 5872007

Please sign in to comment.