Commit 3cf3d65a authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

Omnibox UI Experiments: Unelide on caret placement or partial-select

This prototypes the idea of uneliding when the user affirmatively
places the caret within the existing URL or makes a partial selection.

The partial selection can be made either via double-click or by
dragging. The unelision happens on mouseup.

It feels pretty good.

Change-Id: I5522a7b6dc8d94fe84c39c8b71995abc5007651a
Reviewed-on: https://chromium-review.googlesource.com/910110Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537813}
parent 12efe1d5
......@@ -8,6 +8,7 @@
#include <utility>
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
......@@ -126,6 +127,7 @@ OmniboxViewViews::OmniboxViewViews(OmniboxEditController* controller,
delete_at_end_pressed_(false),
location_bar_view_(location_bar),
ime_candidate_window_open_(false),
is_mouse_pressed_(false),
select_all_on_mouse_release_(false),
select_all_on_gesture_tap_(false),
latency_histogram_state_(NOT_ACTIVE),
......@@ -520,6 +522,39 @@ void OmniboxViewViews::ClearAccessibilityLabel() {
friendly_suggestion_text_prefix_length_ = 0;
}
bool OmniboxViewViews::UnapplySteadyStateElisions() {
if (!base::FeatureList::IsEnabled(
omnibox::kUIExperimentHideSteadyStateUrlSchemeAndSubdomains)) {
return false;
}
// If the mouse is pressed, we defer this operation until mouseup.
if (is_mouse_pressed_)
return false;
// No need to update the text if the user is already inputting text or if
// everything is selected.
if (model()->user_input_in_progress() || IsSelectAll())
return false;
model()->SetInputInProgress(true);
base::string16 full_url = model()->GetCurrentPermanentUrlText();
size_t start, end;
GetSelectionBounds(&start, &end);
// TODO(tommycli): Before this code goes into production, investigate
// whether using the offsets provided by FormatURL makes more sense.
size_t offset = full_url.find(GetText());
if (offset != base::string16::npos) {
start += offset;
end += offset;
}
SetTextAndSelectedRange(full_url, gfx::Range(start, end));
return true;
}
void OmniboxViewViews::OnBeforePossibleChange() {
// Record our state.
GetState(&state_before_change_);
......@@ -542,9 +577,15 @@ bool OmniboxViewViews::OnAfterPossibleChange(bool allow_keyword_ui_change) {
state_changes.text_differs ||
(ime_composing_before_change_ != IsIMEComposing());
const bool something_changed = model()->OnAfterPossibleChange(
bool something_changed = model()->OnAfterPossibleChange(
state_changes, allow_keyword_ui_change && !IsIMEComposing());
// If any mouse button is pressed, we defer this operation until mouseup.
if (UnapplySteadyStateElisions()) {
something_changed = true;
state_changes.text_differs = true;
}
// If only selection was changed, we don't need to call model()'s
// OnChanged() method, which is called in TextChanged().
// But we still need to call EmphasizeURLComponents() to make sure the text
......@@ -642,6 +683,8 @@ const char* OmniboxViewViews::GetClassName() const {
}
bool OmniboxViewViews::OnMousePressed(const ui::MouseEvent& event) {
is_mouse_pressed_ = true;
select_all_on_mouse_release_ =
(event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) &&
(!HasFocus() || (model()->focus_state() == OMNIBOX_FOCUS_INVISIBLE));
......@@ -686,6 +729,10 @@ void OmniboxViewViews::OnMouseReleased(const ui::MouseEvent& event) {
SelectAll(true);
}
select_all_on_mouse_release_ = false;
is_mouse_pressed_ = false;
if (UnapplySteadyStateElisions())
TextChanged();
}
void OmniboxViewViews::OnGestureEvent(ui::GestureEvent* event) {
......
......@@ -144,6 +144,10 @@ class OmniboxViewViews : public OmniboxView,
void ClearAccessibilityLabel();
// Returns true if the user text was updated with the full URL (without
// steady-state elisions).
bool UnapplySteadyStateElisions();
// OmniboxView:
void SetWindowTextAndCaretPos(const base::string16& text,
size_t caret_pos,
......@@ -257,6 +261,9 @@ class OmniboxViewViews : public OmniboxView,
// on Chrome OS.
bool ime_candidate_window_open_;
// True if any mouse button is currently depressed.
bool is_mouse_pressed_;
// Should we select all the text when we see the mouse button get released?
// We select in response to a click that focuses the omnibox, but we defer
// until release, setting this variable back to false if we saw a drag, to
......
......@@ -238,9 +238,7 @@ GURL OmniboxEditModel::PermanentURL() const {
}
base::string16 OmniboxEditModel::GetCurrentPermanentUrlText() const {
// TODO(tommycli): The focus state is a rough approximation, but we will
// need to make this more sophisticated.
if (has_focus())
if (user_input_in_progress_)
return url_for_editing_;
return display_only_url_;
......@@ -1108,7 +1106,7 @@ void OmniboxEditModel::OnPopupDataChanged(
view_->OnInlineAutocompleteTextCleared();
const base::string16& user_text =
user_input_in_progress_ ? user_text_ : url_for_editing_;
user_input_in_progress_ ? user_text_ : GetCurrentPermanentUrlText();
if (keyword_state_changed && is_keyword_selected()) {
// If we reach here, the user most likely entered keyword mode by inserting
// a space between a keyword name and a search string (as pressing space or
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment