New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 651016 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Text cursor is missing while pasting any text in omnibox

Project Member Reported by sc00335...@techmahindra.com, Sep 28 2016

Issue description

Version:  55.0.2873.0 dev
OS: Ubuntu 14.04,Windows

What steps will reproduce the problem?
(1)Launch chrome >> Copy text and paste it in omnibox and observe for text cursor at end

Expected: On pasting Text cursor should be seen.
Actual: Instead no cursor is seen on pasting.

This is a regression broken in M55 as same is working fine in 54.0.2840.41 beta.
 
Expected_text cursor.ogv
430 KB View Download
Actual_text cursor.ogv
343 KB View Download
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on windows 7 using chrome version 55.0.2873.0.the same is working fine on Mac 10.11.6.


Labels: -Needs-Bisect hasbisect-per-revision
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)
Using the per-revision bisect providing the bisect results,
Good build: 55.0.2868.0 (Revision: 420217).
Bad build: 55.0.2870.0 (Revision: 420804).

You are probably looking for a change made after 420666 (known good), but no later than 420667 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/d9adeff125559612a223819de5fce36c5e337b4f..1c77b2b6c04efe4e5d5d3ec5688616d65b8d0138


@ellyjones -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.
Thank You.
Labels: ReleaseBlock-Beta
Adding beta blocker as this is recent regression.Please feel free to adjust if not required.
Status: Started (was: Assigned)
https://codereview.chromium.org/2373403002/

Comment 5 by yosin@chromium.org, Sep 29 2016

Components: -Blink>Editing>Paste
Blink doesn't affect omonibox.

Comment 6 by gov...@chromium.org, Sep 29 2016

This bug is reported as M55 Beta blocker.Please try to resolve this before M55 branch on Oct 6th,2016 so it has enough baking time in Dev.


Comment 7 by ajha@chromium.org, Oct 3 2016

ellyjones@: Could you please take a look into the CL review under progress and if possible, land the fix before M-55 branch as per C#6.


A friendly reminder that M55 Beta launch is coming soon! Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.
Cc: kathrelk...@chromium.org
@kathrelkeld: Katherine, when this change lands could you do a quick check on Chrome OS that the Omnibox text editing doesn't seem to regress? Things to check might be ChromeVox feedback and cursor highlighting as you select with the mouse or keyboard, or cut and paste text. Hopefully nothing should have changed.

Thanks!


(I am worried, for example, about getting extra spurious announcements of text changes from ChromeVox.)
#9: We'll be sure to double check this during our FullRelease test pass next week.

**** Bulk edit -  please ignore if not applicable ****

This bug  is reported as M55 Beta blocker and we're getting closer to M55 Beta promotion. 
Please plan to have fix ready and merged to M55 branch (2883) by 5:00 PM PT, Monday(10/10) so it has enough baking time in Dev before Beta promotion. Thank you.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 7 2016

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

commit cb06e3bec608cfc5f294d4aaa1a2c2e983017f05
Author: ellyjones <ellyjones@chromium.org>
Date: Fri Oct 07 19:33:00 2016

views: call UpdateAfterChange() in Textfield::InsertOrReplaceText().

The Omnibox uses InsertOrReplaceText() to implement its own paste handling that bypasses Textfield::OnPaste(), but the cursor still needs to be kept in sync with pastes.

BUG= 651016 

Review-Url: https://codereview.chromium.org/2399863002
Cr-Commit-Position: refs/heads/master@{#423930}

[modify] https://crrev.com/cb06e3bec608cfc5f294d4aaa1a2c2e983017f05/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/cb06e3bec608cfc5f294d4aaa1a2c2e983017f05/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/cb06e3bec608cfc5f294d4aaa1a2c2e983017f05/ui/views/controls/textfield/textfield_test_api.h
[modify] https://crrev.com/cb06e3bec608cfc5f294d4aaa1a2c2e983017f05/ui/views/controls/textfield/textfield_unittest.cc

Please request a merge to M55 if change is baked/verified in Canary and safe to merge. Thank you.
Labels: Merge-Request-55

Comment 16 by dimu@chromium.org, Oct 12 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 12 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2d6905a80d1603373c401558a145e195714938d

commit b2d6905a80d1603373c401558a145e195714938d
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Oct 12 20:36:26 2016

views: call UpdateAfterChange() in Textfield::InsertOrReplaceText().

The Omnibox uses InsertOrReplaceText() to implement its own paste handling that bypasses Textfield::OnPaste(), but the cursor still needs to be kept in sync with pastes.

BUG= 651016 

Review-Url: https://codereview.chromium.org/2399863002
Cr-Commit-Position: refs/heads/master@{#423930}
(cherry picked from commit cb06e3bec608cfc5f294d4aaa1a2c2e983017f05)

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

Cr-Commit-Position: refs/branch-heads/2883@{#71}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield_test_api.h
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield_unittest.cc

Status: Fixed (was: Started)
Merge complete to 2883.
Labels: TE-Verified-M55 TE-Verified-55.0.2883.11
Verified the issue on Windows-10 and Linux ubuntu 14.04 using chrome version 55.0.2883.11 with the steps mentioned above. Fix is working fine. Please find the attached screencast.
Win-651016.mp4
450 KB View Download
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2d6905a80d1603373c401558a145e195714938d

commit b2d6905a80d1603373c401558a145e195714938d
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Oct 12 20:36:26 2016

views: call UpdateAfterChange() in Textfield::InsertOrReplaceText().

The Omnibox uses InsertOrReplaceText() to implement its own paste handling that bypasses Textfield::OnPaste(), but the cursor still needs to be kept in sync with pastes.

BUG= 651016 

Review-Url: https://codereview.chromium.org/2399863002
Cr-Commit-Position: refs/heads/master@{#423930}
(cherry picked from commit cb06e3bec608cfc5f294d4aaa1a2c2e983017f05)

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

Cr-Commit-Position: refs/branch-heads/2883@{#71}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield_test_api.h
[modify] https://crrev.com/b2d6905a80d1603373c401558a145e195714938d/ui/views/controls/textfield/textfield_unittest.cc

Comment 21 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment