Issue 6285003: DOMUI: Use dispatchSimpleEvent instead of programatically clicking the checkbox

Issue 6285003: DOMUI: Use dispatchSimpleEvent instead of programatically clicking the checkbox (Closed)

Created:
9 years, 11 months ago by James Hawkins

Modified:
9 years, 7 months ago

Reviewers:
stuartmorgan, arv (Not doing code reviews)

CC:
chromium-reviews, arv (Not doing code reviews)

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

DOMUI: Use dispatchSimpleEvent instead of programatically clicking the checkbox to notify the control that the checked state has changed. BUG=69613 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71973

Patch Set 1 #

Total comments: 2

Patch Set 2 : Review fix. #

Total comments: 1

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+12 lines, -6 lines)			Patch
	M	chrome/browser/resources/options/personal_options.js	View	1	1 chunk	+10 lines, -4 lines	1 comment	Download
	M	chrome/browser/resources/options/pref_ui.js	View		2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

stuartmorgan

http://codereview.chromium.org/6285003/diff/1/chrome/browser/resources/options/personal_options.js File chrome/browser/resources/options/personal_options.js (right): http://codereview.chromium.org/6285003/diff/1/chrome/browser/resources/options/personal_options.js#newcode116 chrome/browser/resources/options/personal_options.js:116: cr.dispatchSimpleEvent(syncCheckboxes[i], 'change') Shouldn't you only send this if it ...

9 years, 11 months ago (2011-01-20 18:19:42 UTC) #3

James Hawkins

9 years, 11 months ago (2011-01-20 18:25:12 UTC) #4

arv (Not doing code reviews)

Sorry for being late... http://codereview.chromium.org/6285003/diff/6001/chrome/browser/resources/options/personal_options.js File chrome/browser/resources/options/personal_options.js (right): http://codereview.chromium.org/6285003/diff/6001/chrome/browser/resources/options/personal_options.js#newcode116 chrome/browser/resources/options/personal_options.js:116: cr.dispatchSimpleEvent(syncCheckboxes[i], 'change'); This seems backwards. ...

9 years, 11 months ago (2011-01-21 23:57:29 UTC) #6

James Hawkins

9 years, 11 months ago (2011-01-25 22:42:21 UTC) #7

On 2011/01/21 23:57:29, arv wrote:
> Sorry for being late...
> 
>
http://codereview.chromium.org/6285003/diff/6001/chrome/browser/resources/opt...
> File chrome/browser/resources/options/personal_options.js (right):
> 
>
http://codereview.chromium.org/6285003/diff/6001/chrome/browser/resources/opt...
> chrome/browser/resources/options/personal_options.js:116:
> cr.dispatchSimpleEvent(syncCheckboxes[i], 'change');
> This seems backwards.
> 
> We should not dispatch fake events with the purpose to get the handler
invoked.
> Instead we should just call the method we care about.

There checkboxes were removed in M10, so I'll keep this in mind when
re-implementing the sync section in M11.

Expand Messages | Collapse Messages