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

Issue 682120 link

Starred by 1 user
Status: Verified
Owner:
michae...@chromium.org
Closed: Feb 2017
Cc:
█ hdodda@chromium.org
nyerramilli@chromium.org
█ msrchandra@chromium.org
michae...@chromium.org
jmukthavaram@chromium.org
█ ranjitkan@chromium.org
dpa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: In chrome://md-settings, unnecessary Move up and Move down options are given for first and last language in Language section.

Reported by lpa...@etouch.net, Jan 18 2017 
Chrome Version: 57.0.2984.0 (Official Build) canary a2377c097a1fa051f90f9c02d96d010073fcb701-refs/heads/master@{#443964} (32/64-bit)
OS: Windows (7,8,10), Mac (10.11.6,10.12.1), Linux (14.04 LTS)

What steps will reproduce the problem?
1) Launch chrome, navigate to chrome://md-settings, scroll down the page and click on Advanced.
2) Under Languages, click on Language dropdown and then click on iron icon for first option and select ‘Move up’.
3) Now click on iron icon for last language and select ‘Move down’ and observe.

Unnecessary Move up and Move down options are given for first and last language respectively (As nothing happens on clicking).

Move up and Move down options should be disabled for first and last languages respectively.

This is a Regression issue broken in M-57, will soon update other info
Manual bisect:
Good build: 57.0.2950.0
Bad build: 57.0.2951.0
languages_actual.mov
3.0 MB Download
language_expected.mov
3.2 MB Download

Comment 1 by hdodda@chromium.org, Jan 18 2017

Cc: hdodda@chromium.org
Labels: hasbisect-per-revision
Owner: michae...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 57.0.2950.0 (revision : 438011)
Bad build: 57.0.2952.0 (revision : 438707)

You are probably looking for a change made after 438402 (known good), but no later than 438403 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.
 
https://chromium.googlesource.com/chromium/src/+log/b6f207ca201f3eb08c2150f5c3610c68a30c8984..4cb2126002cd187035ba90ee13ebda77e16d81f6

From the CL above, assigning the issue to the concern owner 

@michaelpg - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Review-Url: https://codereview.chromium.org/2573643005

Thanks!

Comment 2 by steve...@chromium.org, Jan 31 2017

Labels: -OS-Linux -Pri-1 -M-57 M-58 OS-Chrome Pri-2
Owner: steve...@chromium.org
I can take this, it should be a simple fix. Not a P1.

Comment 3 by michae...@chromium.org, Feb 1 2017 

I see what the problem is. I can tackle it tomorrow, if you don't mind. I also should have added a test for this and the Move to Top behavior.

Comment 4 by steve...@chromium.org, Feb 1 2017

Owner: michae...@chromium.org
Sounds good, cheers.

Comment 5 by michae...@chromium.org, Feb 1 2017

Cc: jmukthavaram@chromium.org michae...@chromium.org
 Issue 687468  has been merged into this issue.

Comment 6 by michae...@chromium.org, Feb 1 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 2 2017 

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

commit 4fed887018309685829be2b323c799ff1bf13c60
Author: michaelpg <michaelpg@chromium.org>
Date: Thu Feb 02 20:57:19 2017

Language settings: Fix unnecessarily shown move buttons

Each language in the list of enabled languages has a detail menu with options
like "move up" and "move down". Obviously some options should be hidden, e.g.
the top language's detail menu should not show "move up" or "move to top". This
was broken by me in crrev.com/2573643005.

Fix by comparing the Language object, whose identity never changes for a given
language. The breakage happened because the LanguageState object being compared
was now being copied to a new object.

In the future we should simplify language settings by flattening the Language
structure into the LanguageState structure instead of having "language.language"
be a thing.

BUG= 682120 
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/4fed887018309685829be2b323c799ff1bf13c60/chrome/browser/resources/settings/languages_page/languages_page.html
[modify] https://crrev.com/4fed887018309685829be2b323c799ff1bf13c60/chrome/browser/resources/settings/languages_page/languages_page.js
[modify] https://crrev.com/4fed887018309685829be2b323c799ff1bf13c60/chrome/test/data/webui/settings/languages_page_browsertest.js

Comment 8 by michae...@chromium.org, Feb 3 2017

Status: Fixed (was: Started)

Comment 9 by rookrishna@chromium.org, Feb 10 2017

Status: Verified (was: Fixed)
►

Sign in to add a comment