Closed Bug 1197394 Opened 7 years ago Closed 7 years ago

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

Categories

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

defect
Not set
normal

Tracking

(firefox43 fixed)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: jj, Assigned: jj)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → jj
Status: NEW → ASSIGNED
Attachment #8651257 - Flags: review?(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.
Attachment #8651257 - Flags: review?(jwalker) → review+
Thanks for the patch. I'll uplift it to the GCLI repo.
Attachment #8651257 - Attachment is obsolete: true
Attachment #8651358 - Flags: review?(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!
Attachment #8651358 - Flags: review?(jwalker) → review+
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?
Flags: needinfo?(jwalker)
Just land in m-c, I'll sort GCLI when I'm less busy.
Flags: needinfo?(jwalker)
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.
Flags: needinfo?(jj)
https://hg.mozilla.org/mozilla-central/rev/821f73eb7aef
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.