Language settings: Fix unnecessarily shown move buttons

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}
Committed: https://chromium.googlesource.com/chromium/src/+/4fed887018309685829be2b323c799ff1bf13c60

[michaelpg, 2017-02-01 22:12:39]

Description was changed from

==========
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
==========

to

==========
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
==========

[michaelpg, 2017-02-01 22:31:56]

Verified the test fails without the fix with, e.g.:

> AssertionError: Menu item "moveToTop" hidden: expected false to equal true

[stevenjb, 2017-02-02 00:26:34]

lgtm

https://codereview.chromium.org/2675453002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2675453002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:99: *    
preferred languages.
nit: Put 'Used to show or hide...' at top of comment.

[michaelpg, 2017-02-02 19:38:31]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2017-02-02 19:38:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2675453002/#ps40001
(title: "nit")

[michaelpg, 2017-02-02 19:38:36]

https://codereview.chromium.org/2675453002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2675453002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:99: *    
preferred languages.
On 2017/02/02 00:26:34, stevenjb wrote:
> nit: Put 'Used to show or hide...' at top of comment.

Done.

[commit-bot: I haz the power, 2017-02-02 19:38:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 20:57:23]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486064311224040,
"parent_rev": "137677f4e89b62bb98c8007ee3eb9d9e26de1302", "commit_rev":
"4fed887018309685829be2b323c799ff1bf13c60"}

[commit-bot: I haz the power, 2017-02-02 20:57:58]

Description was changed from

==========
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
==========

to

==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/4fed887018309685829be2b323c7...
==========

[commit-bot: I haz the power, 2017-02-02 20:57:59]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/4fed887018309685829be2b323c7...