Print Preview Cleanup: Creating enum with all custom events used.

Print Preview Cleanup: Creating enum with all custom events used.

BUG=100883
TEST=NONE


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107513

[Evan Stade, 2011-10-25 16:58:34]

denied

this is not an improvement. If you did change the string name for some reason,
you'd also change the var name (I challenge you to think of a change to
PDF_GENERATION_ERROR: 'PDFGenerationError', that doesn't involve changing both
sides of the colon)

[dpapad, 2011-10-25 17:05:23]

On 2011/10/25 16:58:34, Evan Stade wrote:
> denied
> 
> this is not an improvement. If you did change the string name for some reason,
> you'd also change the var name (I challenge you to think of a change to
> PDF_GENERATION_ERROR: 'PDFGenerationError', that doesn't involve changing both
> sides of the colon)

The you are giving is not the reason for making those changes. The reason is to
have a single place where all used events are listed (and documented). There
were occurrences in the past where two different event names where used for the
same event. Also adding a new event (by somebody who is not familiar with the
code) would require to manually search all print preview files to find if such
an event already exists.

[Evan Stade, 2011-10-25 17:16:43]

I was just responding to the bug description. If you want a place for
documentation, then the place where it's dispatched seems like a fine choice to
me (there is no better documentation than context anyway). 

Grepping for dispatchEvent/dispatchSimpleEvent provides the list. How did two
equivalent events coexist? To be equivalent wouldn't they have to be dispatched
from the same place?

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:109: DRAG: 'Drag',
this needs a better name

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:306: * Updates the
controls with printer capabilities information.
this documentation is out of date

[dpapad, 2011-10-25 18:35:32]

Addressed comments.

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:109: DRAG: 'Drag',
On 2011/10/25 17:16:43, Evan Stade wrote:
> this needs a better name

Done.

http://codereview.chromium.org/8357009/diff/7001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.js:306: * Updates the
controls with printer capabilities information.
On 2011/10/25 17:16:43, Evan Stade wrote:
> this documentation is out of date

Done.

[Evan Stade, 2011-10-26 23:16:28]

lgtm notwithstanding that I disagree with this use of events

http://codereview.chromium.org/8357009/diff/10003/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/print_preview.js (right):

http://codereview.chromium.org/8357009/diff/10003/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/print_preview.js:109: MARGINS_LINE_DRAG:
'marginsLineDrag',
no S

[commit-bot: I haz the power, 2011-10-27 00:56:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpapad@chromium.org/8357009/16001

[commit-bot: I haz the power, 2011-10-27 02:05:52]

Change committed as 107513