1197394 - GCLI’s number-type doesn’t accept min/max-value of 0

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

Description

[Johan K. Jensen :johankj]

GCLI’s number-type ignores min or max-values of 0 as seen in bug 970625.

E.g.:
>    type: { name: "number", min: 0 },
won’t have any minimum value, but
>    type: { name: "number", min: 1 },
will have a minimum value of 1.

Comment 1

[Johan K. Jensen :johankj]

Attachment: bug1197394_gcli_numbertype_ignores_min_max_0_value.patch

Comment 2

[Joe Walker [:jwalker]]

Comment on attachment 8651257 [details] [diff] [review]
bug1197394_gcli_numbertype_ignores_min_max_0_value.patch

Review of attachment 8651257 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/gcli/source/lib/gcli/types/number.js
@@ +68,5 @@
>        return '' + value;
>      },
>  
>      getMin: function(context) {
> +      if (this.min != undefined) {

Could you use:

    if (this.min != null) {

instead?

A test for (x != null) is true if x is null or undefined, where using (x != undefined) is only true if x is undefined, also this is the style used in the rest of GCLI code. Really surprised that this is written as it is TBH.

Comment 3

[Joe Walker [:jwalker]]

Thanks for the patch. I'll uplift it to the GCLI repo.

Comment 4

[Johan K. Jensen :johankj]

Attachment: bug1197394_gcli_numbertype_ignores_min_max_0_value.patch

Comment 5

[Joe Walker [:jwalker]]

Comment on attachment 8651358 [details] [diff] [review]
bug1197394_gcli_numbertype_ignores_min_max_0_value.patch

Review of attachment 8651358 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Joe, are you planning to pull this up into GCLI and then back down to Gecko?

Or should this patch land in m-c right now, and you'll handle the GCLI repo separately?

Comment 7

[Joe Walker [:jwalker]]

Just land in m-c, I'll sort GCLI when I'm less busy.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Johan, are you able to push this to try?

I typically use a commit such as:

try: -b do -p linux64,macosx64,win32 -u xpcshell,mochitest-dt,mochitest-o,mochitest-e10s-devtools-chrome

for most DevTools changes.

Comment 9

[Johan K. Jensen :johankj]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43d71b85dd82

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/821f73eb7aef

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/821f73eb7aef