620593 - Username should allow leading and trailing whitespace

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Tony Chung [:tchung]]

Attachment: Whitespace error screenshot

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

Comment 1

[Richard Newman [:rnewman]]

Attachment: fx-sync part. v1

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?)

Comment 2

[Richard Newman [:rnewman]]

Attachment: mozilla-central part. v1

mozilla-central part. This uses the contents of the prior patch to augment the built-in Firefox 4 UI.

Comment 3

[Richard Newman [:rnewman]]

Pushed to fx-sync:

http://hg.mozilla.org/services/fx-sync/rev/76e2cb771615

Comment 4

[Richard Newman [:rnewman]]

Second part of this work depends on merging fx-sync to mozilla-central, so it's on hold for now.

Comment 5

[Philipp von Weitershausen [:philikon]]

(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.

Comment 6

[Richard Newman [:rnewman]]

Reassigning to you, Philipp, so you remember to pull this into m-c.

Comment 7

[Philipp von Weitershausen [:philikon]]

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

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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

Comment 9

[Philipp von Weitershausen [:philikon]]

(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()?

Comment 10

[Richard Newman [:rnewman]]

> 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.

Comment 11

[Richard Newman [:rnewman]]

Attachment: Use built-in trim. v1

xpcshell-tests and xulrunner tests pass, so this works in 3.6. Going to try to find a 3.5 to run tests.

Comment 12

[Richard Newman [:rnewman]]

Tests for this pass in 3.5.16.

Comment 13

[Richard Newman [:rnewman]]

Pushed:

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

Just waiting on the m-c stuff before closing.

Comment 14

[Richard Newman [:rnewman]]

Pushed a while back:

http://hg.mozilla.org/mozilla-central/rev/e65cade94ad9

Comment 15

[Tracy Walker [:tracy]]

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?

Comment 16

[Philipp von Weitershausen [:philikon]]

(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!

Comment 17

[Tracy Walker [:tracy]]

I do think this was fixed for a time I recall checking it.  It has since regressed. filed bug 629782

Comment 18

[Philipp von Weitershausen [:philikon]]

The mozilla-central bits were never pushed, reopening this.

Comment 19

[Richard Newman [:rnewman]]

Transplanted fx-sync part (and followups) to 1.6.x:

changeset:   3299:3d2529479010
branch:      1.6.x
user:        Richard Newman <[e-mail]>
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 <[e-mail]>
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 <[e-mail]>
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

Comment 20

[Philipp von Weitershausen [:philikon]]

Landed m-c part in places: https://hg.mozilla.org/projects/places/rev/21083faea88b

Comment 21

[Tracy Walker [:tracy]]

verified with 1.6.3 on 3.5.16 and 3.6.13

Comment 22

[Philipp von Weitershausen [:philikon]]

Landed on m-c: http://hg.mozilla.org/mozilla-central/rev/21083faea88b

Comment 23

[Tracy Walker [:tracy]]

verified with nightly minefield build of 20110218