Last Comment Bug 675794 - about:config - must strip whitespace as a name for new alphanumeric
: about:config - must strip whitespace as a name for new alphanumeric
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 9
Assigned To: Felix Fung (:felix)
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
Depends on:
Blocks: 687820
  Show dependency treegraph
 
Reported: 2011-08-01 16:14 PDT by sys.sgx
Modified: 2011-09-21 02:52 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
ff5.png (12.49 KB, image/png)
2011-08-01 16:14 PDT, sys.sgx
no flags Details
Trimming config names (1.27 KB, patch)
2011-09-15 15:55 PDT, Felix Fung (:felix)
gavin.sharp: review+
margaret.leibovic: feedback+
Details | Diff | Splinter Review

Description sys.sgx 2011-08-01 16:14:54 PDT
Created attachment 549958 [details]
ff5.png

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:5.0.1) Gecko/20100101 Firefox/5.0.1
Build ID: 20110707182747

Steps to reproduce:

1. Browse to about:config
2. Right-click, and select New -> alphanumeric
3. When asked for a name, just press the spacebar once (for one space char)
4. Name accepted, you can also press spacebar for the value
5. New firefox value has been added, but with no name for it.

What's the point of having something like the one shown in the attached file?
Comment 1 Mihaela Velimiroviciu (:mihaelav) 2011-09-13 02:51:08 PDT
Build identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:9.0a1) Gecko/20110912 Firefox/9.0a1

I was able to reproduce the issue on latest Nightly build: new preference can be added with space as the name.
Comment 2 sys.sgx 2011-09-13 04:36:41 PDT
thanks for the update.
I will reinclude my question:
What's the point of having something like the one shown in the attached file?
Comment 3 Felix Fung (:felix) 2011-09-15 15:55:03 PDT
Created attachment 560480 [details] [diff] [review]
Trimming config names

Summary of Changes
- Trim whitespace from any config name before creating it and if after trimming the preference name is empty, we cancel config creation.
Comment 4 sys.sgx 2011-09-15 16:03:05 PDT
Thanks Felix :-)
I think is ok now.
Comment 5 :Margaret Leibovic 2011-09-16 16:47:29 PDT
Comment on attachment 560480 [details] [diff] [review]
Trimming config names

This looks okay to me, but I don't actually have the authority to review this, so I'm passing it off to Gavin.
Comment 6 :Margaret Leibovic 2011-09-19 10:52:29 PDT
Felix, as an FYI, once a patch is review+, you can set the checkin-needed flag on the bug, and someone will usually come around and land it for you :)
Comment 7 Ed Morley [:emorley] 2011-09-20 03:24:55 PDT
This is now in my queue of things that are being sent to try then onto inbound.

I've fixed it locally and imported the patch fine for now; but so you know for future patches, this one had |exporting patch: <fdopen>| spuriously appended. Your best bet is to use mercurial queues (use as new a version of hg as possible) and follow the suggestions here: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Thanks :-)
Comment 8 sys.sgx 2011-09-20 06:03:33 PDT
=== Update - New issue ===

New keys/values should should also strip *all* invalid characters.
I didnt start a new bug, as this can be fixed under the same topic.

How to reproduce:
1. Right click-> New alphanumeric
2. Enter any key that is a product of Alt+(num), ie ☼, ►, •, etc.

We should change at least the key creation to include only valid key characters.

Thanks
Comment 9 Ed Morley [:emorley] 2011-09-20 06:15:21 PDT
The patch for the whitespace issue already has review/is about to land, so I've broken that out into a separate bug (bug 687820), to keep things clearer.
Comment 11 Marco Bonardo [::mak] 2011-09-21 02:52:00 PDT
https://hg.mozilla.org/mozilla-central/rev/028baeb2ca21

Note You need to log in before you can comment on or make changes to this bug.