gcli string params should allow empty values.

RESOLVED FIXED in Firefox 24

Status

defect
RESOLVED FIXED
6 years ago
9 months ago

People

(Reporter: mgoodwin, Assigned: jwalker)

Tracking

20 Branch
Firefox 24
x86
macOS
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

6 years ago
Issue:
I'm working on a gcli command for which a string parameter defaults to a non empty value, but for which an empty string should be valid. gcli apparently does not allow an empty value to be specified by the user.

STR:
This behaviour can be seen with the 'cookie set' built in command. Try to set a cookie with an empty path:
1) Type 'cookie set foo bar --path ""'
2) Observe the help text in the command bar displaying '<string> [option]'
3) Press enter; observe the helper baloon showing the text "The path of the cookie to set".
There are many cases where a string is required and a blank won't do (for example the "context <name>" command) so we need to provide for the "empty string is invalid" case too.

The question is should we make the cookie command say

          {
            name: "path",
            type: { name: "string", allowBlank: true },
            ...
          },

Or should the context command say:

          {
            name: "name",
            type: { name: "string", forceValue: true },
            ...
          },

I think there are 2 reasons to prefer allowBlank: true

1. It doesn't change the current default
2. Having defaultValue: '' does nearly the same thing, allowing the user to not
   type a value. This is only an issue where there is a string where blank is
   valid, but there is a non-blank defaultValue
Posted patch v1 (obsolete) — Splinter Review
Attachment #749169 - Flags: feedback?(mgoodwin)
For detail on the commits, see the last to entries here:
https://github.com/joewalker/gcli/commits/allowblank-866680
Posted patch v2Splinter Review
Rebase
Attachment #749169 - Attachment is obsolete: true
Attachment #749169 - Flags: feedback?(mgoodwin)
Comment on attachment 750415 [details] [diff] [review]
v2

mgoodwin raised this bug as part of creating security commands, so I've f?ed him.
harth would probably be glad of mgoodwin's f+ before her review.
Attachment #750415 - Flags: review?(fayearthur)
Attachment #750415 - Flags: feedback?(mgoodwin)
Reporter

Comment 7

6 years ago
Comment on attachment 750415 [details] [diff] [review]
v2

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

Looks OK to me.
Attachment #750415 - Flags: feedback?(mgoodwin) → feedback+
Attachment #750415 - Flags: review?(fayearthur) → review+
https://hg.mozilla.org/mozilla-central/rev/97892f7b402a
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24

Updated

Last year
Product: Firefox → DevTools

Updated

9 months ago
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.