Next button on Account Details page does not get enabled

RESOLVED FIXED in seamonkey2.4

Status

SeaMonkey
Sync UI
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.4

SeaMonkey Tracking Flags

(seamonkey2.2 fixed, seamonkey2.3 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
STR:
1. Tools/Set Up Sync
2. Create a New Account
3. enter valid email address, 2x password, check "I agree ..."

Expected:
"Next >" button is enabled.

Actual results:
"Next >" button stays disabled.

Workaround:
Change the Server drop-down to "Use a custom server" and back to "SeaMonkey Sync Server".

This problem does not exist in Firefox, so it was somehow broken through the changes we made to the SeaMonkey port (a.k.a. review). :-(
(Assignee)

Comment 1

6 years ago
Hmm, for some reason this.status.server is false in syncSetup.js (while this.status.password and this.status.email are true) when readyToAdvance is called. Thus the check exits early and returns false on line 217. Firefox uses roughly the same code there, so it's really that this.status.server has the wrong value by then.

this.status.server is set by either onServerCommand or checkServer (the initial value is false of course). The difference between FF and SM is that FF calls onServerCommand from onPageShow (case NEW_ACCOUNT_START_PAGE) while this was removed for SM during review. :-(

I think we need to fix this ASAP. If not for 2.2, then maybe 2.3. The more/earlier, the better.
(Assignee)

Comment 2

6 years ago
The regression comes down to bug 634419 comment 25.
Depends on: 634419
(Assignee)

Comment 3

6 years ago
Created attachment 539676 [details] [diff] [review]
patch

Neil, I'm inclined to fix this the easy way, by undoing your original request and bringing us back in line with what Firefox does (here).
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #539676 - Flags: review?(neil)

Comment 4

6 years ago
So what you're saying is that, unlike the email/password case, the server is initially valid?
(In reply to comment #4)
> So what you're saying is that, unlike the email/password case, the server is
> initially valid?

It's a valid server since it's the default one, but the Next button should only unlock if you accept the ToS.
(Assignee)

Comment 6

6 years ago
(In reply to comment #4)
> So what you're saying is that, unlike the email/password case, the server is
> initially valid?

Yes (like philiKON said), but the init code also needs to do the correct thing if you come back to this page (it's a wizard!) after navigating elsewhere. So I wouldn't like to set this.status.server to true by default, because that might only work for the very initial case.

Comment 7

6 years ago
(In reply to comment #5)
> (In reply to comment #4)
> > So what you're saying is that, unlike the email/password case, the server is
> > initially valid?
> 
> It's a valid server since it's the default one, but the Next button should
> only unlock if you accept the ToS.
Sure, but there's a separate test for that. (In theory the server validity could have been made to depend on the ToS checkbox, but it doesn't.)

(In reply to comment #6)
> Yes (like philiKON said), but the init code also needs to do the correct
> thing if you come back to this page (it's a wizard!) after navigating
> elsewhere. So I wouldn't like to set this.status.server to true by default,
> because that might only work for the very initial case.
I've just double-checked and the page is nicely self-contained and I can't find anything on the other pages that could affect its validity.
(Assignee)

Comment 8

6 years ago
Created attachment 540153 [details] [diff] [review]
patch v2 [Checkin: comments 10 and 11]

OK, just flipping the default seems to do.

I found another issue, though (not directly related): If you change the server type to custom but don't enter anything into the (then) editable field, the Next button gets enabled. I found that control.selectedIndex == -1 in onServerCommand while it's only checked against > 0 (and only in SM; I guess I am to blame for having introduced this in bug 634419).
Attachment #539676 - Attachment is obsolete: true
Attachment #539676 - Flags: review?(neil)
Attachment #540153 - Flags: review?(neil)

Comment 9

6 years ago
Comment on attachment 540153 [details] [diff] [review]
patch v2 [Checkin: comments 10 and 11]

I see all the other cases compare selectedIndex == 0 already.
Attachment #540153 - Flags: review?(neil) → review+
(Assignee)

Comment 10

6 years ago
Comment on attachment 540153 [details] [diff] [review]
patch v2 [Checkin: comments 10 and 11]

http://hg.mozilla.org/comm-central/rev/9685aa9dadd4

Requesting aurora/beta approval for simple and safe patch that 
1. fixes the case where you could not create a Sync account at all unless you found that switching the Sync server back-and-forth helped
2. fixes the case where you could proceed with an empty custom server.
Attachment #540153 - Attachment description: patch v2 → patch v2 [Checkin: comment 10]
Attachment #540153 - Flags: approval-comm-beta?
Attachment #540153 - Flags: approval-comm-aurora?
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.4

Updated

6 years ago
Attachment #540153 - Flags: approval-comm-beta?
Attachment #540153 - Flags: approval-comm-beta+
Attachment #540153 - Flags: approval-comm-aurora?
Attachment #540153 - Flags: approval-comm-aurora+
(Assignee)

Comment 11

6 years ago
Comment on attachment 540153 [details] [diff] [review]
patch v2 [Checkin: comments 10 and 11]

http://hg.mozilla.org/releases/comm-aurora/rev/49d9e3760168
http://hg.mozilla.org/releases/comm-beta/rev/c9d3e5785364
Attachment #540153 - Attachment description: patch v2 [Checkin: comment 10] → patch v2 [Checkin: comments 10 and 11]
(Assignee)

Updated

6 years ago
Target Milestone: seamonkey2.4 → seamonkey2.2

Updated

6 years ago
status-seamonkey2.2: --- → fixed
status-seamonkey2.3: --- → fixed
Target Milestone: seamonkey2.2 → seamonkey2.4
You need to log in before you can comment on or make changes to this bug.