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

Issue 1827823003: MD Settings: Rename and fix confusing prefs test function (Closed)

Created:
4 years, 9 months ago by michaelpg
Modified:
4 years, 5 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@PrefsTests
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MD Settings: Rename and fix confusing prefs test function The misleading "resetForTesting" actually just partially uninitialized the element, making it behave unexpectedly in tests. The function has been expanded into "uninitializeForTesting" which correctly un-initializes the element. See also https://codereview.chromium.org/1833613002#ps1 for more fallout from this confusion.

Patch Set 1 #

Patch Set 2 : simplify #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M chrome/browser/resources/settings/prefs/prefs.js View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/test/data/webui/settings/prefs_tests.js View 1 chunk +1 line, -1 line 0 comments Download

Depends on Patchset:

Messages

Total messages: 8 (5 generated)
michaelpg
4 years, 9 months ago (2016-03-24 03:31:12 UTC) #5
michaelpg
simplified
4 years, 9 months ago (2016-03-24 04:05:41 UTC) #7
Dan Beam
4 years, 9 months ago (2016-03-24 19:08:42 UTC) #8
i don't think this is actually clearer

Powered by Google App Engine
This is Rietveld 408576698