Change print setting conventions for margins to support more options.

Change print setting conventions for margins to support more options.

BUG=NONE
TEST=NONE

[dpapad, 2011-10-12 15:59:05]

http://codereview.chromium.org/8227034/diff/5001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/margin_settings.js (left):

http://codereview.chromium.org/8227034/diff/5001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/margin_settings.js:69: return
this.selectedMarginsValue_ == this.defaultMarginsValue_;
After landing http://codereview.chromium.org/7891016/, these changes are
conflicting. I would recommend adding method get marginTypeString without
removing isDefaultMarginsSelected, isNoMarginsSelected and
isCustomMarginsSelected.

[vandebo (ex-Chrome), 2011-10-12 17:32:49]

http://codereview.chromium.org/8227034/diff/5001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/margin_settings.js (left):

http://codereview.chromium.org/8227034/diff/5001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/margin_settings.js:69: return
this.selectedMarginsValue_ == this.defaultMarginsValue_;
On 2011/10/12 15:59:05, dpapad wrote:
> After landing http://codereview.chromium.org/7891016/, these changes are
> conflicting. I would recommend adding method get marginTypeString without
> removing isDefaultMarginsSelected, isNoMarginsSelected and
> isCustomMarginsSelected.

Agreed. Done.

[dpapad, 2011-10-12 18:04:22]

LGTM, with one comment.

http://codereview.chromium.org/8227034/diff/7001/printing/print_job_constants.cc
File printing/print_job_constants.cc (right):

http://codereview.chromium.org/8227034/diff/7001/printing/print_job_constants...
printing/print_job_constants.cc:105: const char kSettingMarginsTypeCustom[] =
"customMargins";
Do kSettingsMarginsType{Custom|None|Default} have to be strings? Asking because
otherwise you could use the existing method
marginSettings.selectedMarginsValue_, which returns an int depending on the
selected margins type and there would be no need for selectedMarginsAsString().

[vandebo (ex-Chrome), 2011-10-12 20:50:02]

http://codereview.chromium.org/8227034/diff/7001/printing/print_job_constants.cc
File printing/print_job_constants.cc (right):

http://codereview.chromium.org/8227034/diff/7001/printing/print_job_constants...
printing/print_job_constants.cc:105: const char kSettingMarginsTypeCustom[] =
"customMargins";
On 2011/10/12 18:04:22, dpapad wrote:
> Do kSettingsMarginsType{Custom|None|Default} have to be strings? Asking
because
> otherwise you could use the existing method
> marginSettings.selectedMarginsValue_, which returns an int depending on the
> selected margins type and there would be no need for
selectedMarginsAsString().

Nope, changed to an int (taken from an enum, which I was going to introduce in
the next CL anyway).

[commit-bot: I haz the power, 2011-10-13 03:28:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vandebo@chromium.org/8227034/14001

[commit-bot: I haz the power, 2011-10-13 03:28:47]

Can't process patch for file chrome/renderer/print_web_view_helper.cc.
File's status is None, patchset upload is incomplete.