Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1323)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1132223005: Fix the pref binding by binding to the object rather than the element (Closed)

Created:
5 years, 7 months ago by Jeremy Klein
Modified:
5 years, 7 months ago
Reviewers:
michaelpg
CC:
chromium-reviews, khorimoto+watch-md-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, jhawkins+watch-md-settings_chromium.org, orenb+watch-md-settings_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch-md-settings_chromium.org, jlklein+watch-md-settings_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@fixes
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix the pref binding by binding to the object rather than the element BUG=485381 Committed: https://crrev.com/fa6a64ed3ffa3fa27c544478782e14d8642f94a5 Cr-Commit-Position: refs/heads/master@{#329789}

Patch Set 1 #

Patch Set 2 : A couple small fixes #

Patch Set 3 : revert accidental change #

Total comments: 4

Patch Set 4 : ::change #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -80 lines) Patch
M chrome/browser/resources/settings/a11y_page/a11y_page.html View 1 2 3 2 chunks +12 lines, -11 lines 0 comments Download
M chrome/browser/resources/settings/a11y_page/a11y_page.js View 1 2 3 2 chunks +0 lines, -13 lines 0 comments Download
M chrome/browser/resources/settings/checkbox/checkbox.js View 1 chunk +4 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/date_time_page/date_time_page.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/date_time_page/date_time_page.js View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/downloads_page/downloads_page.html View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/settings/downloads_page/downloads_page.js View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/prefs/prefs.js View 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/resources/settings/search_engines_page/search_engines_page.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/search_page/search_page.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/settings.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/settings.js View 1 1 chunk +1 line, -15 lines 0 comments Download
M chrome/browser/resources/settings/settings_main/settings_main.html View 1 chunk +2 lines, -4 lines 0 comments Download
M ui/webui/resources/cr_elements/v0_8/cr_checkbox/cr_checkbox.js View 1 1 chunk +1 line, -8 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
Jeremy Klein
5 years, 7 months ago (2015-05-13 23:20:46 UTC) #2
michaelpg
Overall lgtm (yay for removing redundant settings object) https://codereview.chromium.org/1132223005/diff/40001/chrome/browser/resources/settings/a11y_page/a11y_page.html File chrome/browser/resources/settings/a11y_page/a11y_page.html (right): https://codereview.chromium.org/1132223005/diff/40001/chrome/browser/resources/settings/a11y_page/a11y_page.html#newcode54 chrome/browser/resources/settings/a11y_page/a11y_page.html:54: value="{{prefs.settings.a11y.autoclick_delay_ms}}"> ...
5 years, 7 months ago (2015-05-13 23:49:50 UTC) #3
Jeremy Klein
https://codereview.chromium.org/1132223005/diff/40001/chrome/browser/resources/settings/a11y_page/a11y_page.html File chrome/browser/resources/settings/a11y_page/a11y_page.html (right): https://codereview.chromium.org/1132223005/diff/40001/chrome/browser/resources/settings/a11y_page/a11y_page.html#newcode54 chrome/browser/resources/settings/a11y_page/a11y_page.html:54: value="{{prefs.settings.a11y.autoclick_delay_ms}}"> On 2015/05/13 23:49:50, michaelpg wrote: > add ::change ...
5 years, 7 months ago (2015-05-14 00:53:52 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1132223005/60001
5 years, 7 months ago (2015-05-14 01:06:20 UTC) #7
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 7 months ago (2015-05-14 03:08:53 UTC) #8
commit-bot: I haz the power
5 years, 7 months ago (2015-05-14 03:09:43 UTC) #9
Message was sent while issue was closed. 
Patchset 4 (id:??) landed as
https://crrev.com/fa6a64ed3ffa3fa27c544478782e14d8642f94a5
Cr-Commit-Position: refs/heads/master@{#329789}

Powered by Google App Engine
This is Rietveld 408576698