New issue
Advanced search Search tips

Issue 735609 link

Starred by 1 user
Status: Fixed
Owner:
changwan@chromium.org
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Clean textDeleted selection hack

Project Member Reported by aelias@chromium.org, Jun 21 2017 
In LocationBarLayout.onTextChangedForAutocomplete, there is a hack I suspect is no longer needed especially in the new model.  Filing this to track removal.

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java?q=onTextChangedForAutocomplete&dr=CSs&l=1184

        // Occasionally, was seeing the selection in the URL not being cleared during
        // very rapid editing.  This is here to hopefully force a selection reset during
        // deletes.
        if (textDeleted) mUrlBar.setSelection(mUrlBar.getSelectionStart());
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 22 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d43235d10d2a57e93e417e1ecc5cb430d4136881

commit d43235d10d2a57e93e417e1ecc5cb430d4136881
Author: Changwan Ryu <changwan@chromium.org>
Date: Thu Jun 22 16:54:33 2017

Remove textDeleted from LocationBarLayout

shouldAutocomplete() already tracks whether the last selection was a deletion,
So the only logic change is in mRequestSuggestions.run() that it no longer
checks the old state, which makes sense in case you delete something
and then type immediately.

This also helps simplify the new model we are about to add.

BUG= 735609 ,  539536 

Change-Id: Id5346399e60f4c9e401cc1fe8088c3876fd46c02
Reviewed-on: https://chromium-review.googlesource.com/544516
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481567}
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModel.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivityLocationBarLayout.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/java/src/org/chromium/chrome/browser/toolbar/CustomTabToolbar.java
[modify] https://crrev.com/d43235d10d2a57e93e417e1ecc5cb430d4136881/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

Comment 2 by changwan@chromium.org, Jun 22 2017

Status: Fixed (was: Assigned)
I think moving it into AutocompleteEditTextModel was enough. Thanks for pointing this out. Closing.

Comment 3 by aelias@chromium.org, Jun 22 2017 

Yes, moving it there is fine.
►

Sign in to add a comment