Closed Bug 620593 Opened 9 years ago Closed 9 years ago

Username should allow leading and trailing whitespace

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: tchung, Assigned: philikon)

References

Details

(Whiteboard: [sync1.6])

Attachments

(4 files)

similiar to bug 619567, but we fail when entering username / email address.   Screenshot added.

Repro:
1) install sync 1.6 extension
2) type in existing account, and enter in whitespaces, then your username and password
3) verify incorrect username error

Expected:
- strip whitespaces and validate

Actual
- error
Attached patch fx-sync part. v1Splinter Review
Here's part one:

* Introduce Utils.trim
* Implement Utils.normalizeAccount
* Tests
* Use in Firefox UI. (Do we need to add to others, given the lack of copy/paste?)
Attachment #498959 - Flags: review?(philipp)
mozilla-central part. This uses the contents of the prior patch to augment the built-in Firefox 4 UI.
Attachment #498961 - Flags: review?(philipp)
Attachment #498959 - Flags: review?(philipp) → review+
Attachment #498961 - Flags: review?(philipp) → review+
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Second part of this work depends on merging fx-sync to mozilla-central, so it's on hold for now.
(In reply to comment #4)
> Second part of this work depends on merging fx-sync to mozilla-central, so it's
> on hold for now.

I can get it next time I land stuff on m-c.
Reassigning to you, Philipp, so you remember to pull this into m-c.
Assignee: rnewman → philipp
Blocks: 621194
Comment on attachment 498959 [details] [diff] [review]
fx-sync part. v1

>-        let email    = document.getElementById("weaveEmail").value;
>+        let email    = Weave.Service.normalizeAccount(document.getElementById("weaveEmail").value);

This should've been Weave.Utils.normalizeAccount. It broke the setup wizard. Sigh, I should've caught this in the review, but really what we need is UI tests...

Pushed fix: https://hg.mozilla.org/services/fx-sync/rev/a5c2af879a70
(In reply to comment #8)
> Why are you using a custom Util.trim function instead of the native trim
> method?
> https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/String/Trim

That's a good point! For some reason I thought it was not available in Firefox 3.5 (which we still ahve to support), but I just checked and it is.

Richard, do you want to post an updated patch that uses String.trim()?
> That's a good point! For some reason I thought it was not available in Firefox
> 3.5 (which we still ahve to support), but I just checked and it is.

Heh, I thought exactly the same. Thanks for pointing this out, Vivien!

> Richard, do you want to post an updated patch that uses String.trim()?

Will do.
xpcshell-tests and xulrunner tests pass, so this works in 3.6. Going to try to find a 3.5 to run tests.
Attachment #500059 - Flags: review?(philipp)
Tests for this pass in 3.5.16.
Attachment #500059 - Flags: review?(philipp) → review+
Pushed:

http://hg.mozilla.org/services/fx-sync/rev/04c7972c179a

Just waiting on the m-c stuff before closing.
Attachment #498961 - Flags: approval2.0?
Attachment #498961 - Flags: approval2.0? → approval2.0+
Pushed a while back:

http://hg.mozilla.org/mozilla-central/rev/e65cade94ad9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
with 1.6.2 on 3.6.14:

-In initial account setup, if I prefix and/or suffix the email address with spaces, the ui tells me it's an invalid address.

-If I do the same in the add client wizard, it tells me the user or password is wrong.

Reopen this or file a new bug?
(In reply to comment #15)
> with 1.6.2 on 3.6.14:
> 
> -In initial account setup, if I prefix and/or suffix the email address with
> spaces, the ui tells me it's an invalid address.
> 
> -If I do the same in the add client wizard, it tells me the user or password is
> wrong.
> 
> Reopen this or file a new bug?

Please file a new bug ("Addon UI: ...") if this doesn't affect Firefox 4. Thanks!
I do think this was fixed for a time I recall checking it.  It has since regressed. filed bug 629782
Status: RESOLVED → VERIFIED
The mozilla-central bits were never pushed, reopening this.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Transplanted fx-sync part (and followups) to 1.6.x:

changeset:   3299:3d2529479010
branch:      1.6.x
user:        Richard Newman <rnewman@mozilla.com>
date:        Tue Dec 21 15:32:58 2010 -0800
summary:     Bug 620593: add normalizeAccount, use in addon UI. r=philiKON

changeset:   3300:4fe21d80787c
branch:      1.6.x
user:        Philipp von Weitershausen <philipp@weitershausen.de>
date:        Mon Dec 27 12:48:11 2010 -0800
summary:     Bug 620593 follow-up fix: normalizeAccount is exposed by Weave.Utils, not Weave.Service

changeset:   3301:b92814f21624
branch:      1.6.x
tag:         tip
user:        Richard Newman <rnewman@mozilla.com>
date:        Tue Dec 28 10:33:23 2010 -0800
summary:     Bug 620593: use built-in trim function. r=philiKON


Final diff:

http://hg.mozilla.org/services/fx-sync/rev/b92814f21624
Landed m-c part in places: https://hg.mozilla.org/projects/places/rev/21083faea88b
Whiteboard: [sync1.6] → [sync1.6][fixed in places]
verified with 1.6.3 on 3.5.16 and 3.6.13
Landed on m-c: http://hg.mozilla.org/mozilla-central/rev/21083faea88b
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [sync1.6][fixed in places] → [sync1.6]
verified with nightly minefield build of 20110218
Status: RESOLVED → VERIFIED
Blocks: 684536
No longer blocks: 615950
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.