302 bytes, text/html
272 bytes, text/html
348.78 KB, image/png
3.09 MB, video/quicktime
46 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
For the mail app, "Other Email": Once I finally get to the manual setup option (really ought to be able to skip the 120second will-usually-fail helpful setup option), there's no way to enter a different password for the SMTP option. For my mail, i have different credentials for the IMAP server and the outbound SMTP server. I suspect that a lot of folks who have personal mail boxes, but are on systems that restrict SMTP (like Comcast, Verizon and others) may be in the same situation. It would also be very helpful to be able to know what failed other than "Unknown error". Thanks!
Not a common usage to have different password
Can you elaborate on your specific mail setup? I knew this day would come, but it would be useful to have very specific details on what the account setup is and why it's that way. Although it's very common to block port 25, port 465 is usually not blocked, so I would expect split account setups (retrieve mail from ISP A but send via ISP B) to be super rare. And I have been hoping that there are no ISPs that require a different form of credential between their own IMAP and SMTP servers. Also note that we currently are unable to support SSL certificate exceptions, which are also the type of thing that tends to happen in more complicated situations. "unknown" is indeed pretty useless and I am hoping we are able to improve the strings for v1 before the freeze. Running "adb logcat" to see the console output may shed additional light on the problem, at least if dump() output is enabled.
I'm willing to admit that the account i wanted to set up was a special case. the account name was something like: user: firstname.lastname@example.org imap: mail.example.org smtp: mail.example.com < this is my hosting provider's mail server. I've previously had to use a similar configuration when I was a subscriber with SBC. My email address at the time was @sbc-yahoo.net, but the SMTP server was smtp.sbc.com. They also had blocked all outbound SMTP port (including 465) to force customers to use their server. This was obviously problematic if you didn't use their mail for everything. I stopped being a customer of theirs quite some time ago, so I have no idea if this is still the case.
Thanks for the information! It appears SBC (now AT&T) may no longer block port 465. Specifically, this support article on adding non-AT&T accounts mentions using port 465: http://www.att.com/esupport/article.jsp?sid=KB401601#fbid=Mgu_fZAgqdE The list of blocked on AT&T's wireless networks do not seem to include any mail ports: http://www.att.com/esupport/article.jsp?sid=KB100953&cv=820&_requestid=45366#fbid=Mgu_fZAgqdE It also looks like the older (no longer create-able) SBC accounts have sane domain names now (it's all inbound.att.net/outbound.att.net): http://www.att.com/esupport/article.jsp?sid=KB401570&cv=812&_requestid=44308#fbid=Mgu_fZAgqdE The claim is that the current creatable accounts can't use e-mail clients and have to be accessed at www.att.net, which is very much just a re-branded Yahoo, so standard Yahoo stuff should just work for that. I think this leaves us in the state where we expect that someone out there needs different credentials for different servers, but we don't have an actual problem case yet.
Cool. I'm fine deprioritizing this, then.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
I'll leave this open, but just set this to an enhancement request. I've also tried to update the subject so people who do have the problem will end up here.
Severity: normal → enhancement
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Add password to smtp entry for mail app → Support different username / password / credentials for SMTP server from IMAP server
asuth: I've just tried to set up my email account on B2G and hit this. It makes it impossible to set up this particular account on the B2G mail client. My @mozilla.org email is forwarded to a mailbox at my hosting provider. Mozilla does not, AFAIK, provide email hosting for @mozilla.org addresses. However, because of SPF etc., I have to send email through Mozilla's mail server - which has a different server name, username and password from the IMAP account. Because Thunderbird only has one default SMTP server unless you play with the config, I also use this SMTP server to send personal mail - which means that the different-password problem arises for my personal account as well, which is also stored on the same server. If it were to allow me to proceed without defining an SMTP server, then that might be OK, but it doesn't. In fact, it chunters for a bit and then gives an "Cannot contact server ''" error. If this field is required, I should be told so before leaving the form screen! Gerv
Further checking suggests that the IMAP credentials for my personal account do not work on that provider's SMTP server - I have to use different credentials. So even if I try and use servers from the same org, I hit this problem. This makes it impossible for me to set up either of my email accounts on B2G. Gerv
Gerv, putting aside the credentials issue for a sec, your use-case sounds like what you need is more complicated identity support. Specifically, the UX flow that seems least horrible to me would be: 1) You create your personal e-mail account. 2) You go into the settings UI and add an additional identity associated with your personal account, said identity then also gets its own SMTP server. Maybe you set that identity as your default identity. This would imply we provide a UI affordance to let you switch between accounts/identities on the compose screen. Does this sound like a reasonable way to structure things? In terms of credentials for your personal account, are you saying you have to use a different password? The username field for IMAP and SMTP are distinct, and those are what we use to identify ourselves to the server. The password is the only thing that is required to be the same between them, and that's what this bug is about fixing.
asuth: perhaps there's some confusion; I have no desire to combine these accounts in any way. If Mail only supports one account, I'd only use one - that's fine. I don't want to mix the identities. Yes, I am saying that I use a different password for IMAP vs. SMTP for both my work and personal email arrangements (for different reasons). This bug doesn't sound enormously hard to fix - just move the password field from the top down into the two protocol-specific sections, and prefill both with the password from step 1. Gerv
(In reply to Gervase Markham [:gerv] from comment #10) > asuth: perhaps there's some confusion; I have no desire to combine these > accounts in any way. If Mail only supports one account, I'd only use one - > that's fine. I don't want to mix the identities. Ah, I think I had misunderstood. You're saying you forward email@example.com to gervmoz@gervland and then also have gervpersonal@gervland. I thought they both went to the same place. That does drastically simplify things. Phew! > Yes, I am saying that I use a different password for IMAP vs. SMTP for both > my work and personal email arrangements (for different reasons). Okie dokie. > This bug doesn't sound enormously hard to fix - just move the password field > from the top down into the two protocol-specific sections, and prefill both > with the password from step 1. Yup, this sounds like the right course of action from a UI perspective for manual setup. It's unfortunately a lot more complicated a change than just that though since there are back-end changes involved that need unit tests and the settings UX (which you regrettably haven't seen yet)/settings password change UX/bad-password-detected logic all will need updates and tests. I'm flagging this bug for discussion with the UX team when we meet in a few weeks.
blocking-b2g: --- → koi?
I just tried out Firefox OS using the simulator (version from 2013-07-19) and ran into exactly this bug. Currently there is one field for the password, while the IMAP and SMTP settings have a field for server address and username each. In my case mails arrive at a personal server, which I read using IMAP. I send mails using an account at smtp2go.com. I.e. there is no SMTP Server which accepts the credentials used for the IMAP server. In this case it is impossible to add "the account" at all. Quotes, because there is not one account, but a combination of IMAP and SMTP accounts. It is not clear to me why one should have different usernames for different IMAP and SMTP servers, but not use different passwords for those accounts. Different username, different password. It's easy as that.
triage: koi+ for now -- subject to change if this turns out to be more complex than we think.
blocking-b2g: koi? → koi+
Triage: This bug has been added to the v1.2 backlog but is not blocking release
blocking-b2g: koi+ → -
Tagging this bug to put it in the productivity backlog
tracking-b2g18: --- → 28+
Target Milestone: 1.2 FC (16sep) → ---
Assignee: nobody → mcav
Status: REOPENED → ASSIGNED
Created attachment 8345556 [details] gelam.html Collectively, these patches allow users to specify a separate username/password pair for SMTP accounts. There are a couple of open UX questions and notes below. The GELAM patch adds `outgoingUsername` and `outgoingPassword` fields to accounts' credentials. To ensure that changes to passwords get propagated correctly, I implemented checkAccount() for SMTP, which led to a bit of internal refactoring of SmtpAccount. This should make it straightforward to add "MAIL FROM" probing per bug 948580. The Gaia and UX changes change the manual setup screen to work as follows: - There is now a password field directly below each of the "username" fields for IMAP/POP and SMTP, in addition to the existing one at the very top. This make three password fields total. - When the first password field (global) changes, it updates both the IMAP/POP and SMTP passwords to match. - When the IMAP/POP password is changed, it updates the global password to match. - The SMTP password never affects other fields. UX QUESTIONS: 1) That implementation keeps consistency with our previous implementation, in which users entered a username/password pair, at the expense of potential confusion ("why are there three password fields?"). I initially wrote it this way because it seemed the most consistent with what we do previously, but I could see an argument for only having two password fields during manual account setup, doing away with the weird magic I just described. 2) Secondly, that implementation raises questions about auto-filling the fields in manual setup: I initially thought that if we do have three password fields, that we should auto-fill the username and password for the incoming and outgoing servers, however I now think that's a very bad idea, because some servers require only the pre-"@domain.com" part of the e-mail as the username, and others want the whole thing. Without overcomplicating everything by trying to autodetect the domain (which probably won't work since we're already doing manual setup), auto-filling usernames seems like a bad idea. However, right now the above logic also auto-fills passwords -- that seems more likely to be reasonable, but I'm not sure what's best. 3) There's an important UX flow hidden in here: When you change your password on the server, one or both of your account passwords could be invalid. Say you change your Gmail password -- both passwords will now be invalid, but it doesn't make sense to prompt the user twice (one for IMAP and one for SMTP). To try to avoid overburdening the user with multiple password prompts, the current patch does the following: a) If there was an error connecting to IMAP/POP, prompt for the password. b) If the user's previous IMAP/POP password was the same as their SMTP password, we proactively change _both_ to the new password the user just provided. Then we check the account; if we were correct in assuming that both passwords needed to change, we're done. c) In the off chance we were wrong, and the user now has two _different_ passwords for IMAP/POP and SMTP, we'll notice that the IMAP/POP account worked fine and the SMTP account was still broken -- so we'll prompt for the SMTP password. I don't see a downside to this logic (as opposed to just always prompting twice), but I also don't think iOS does this. The alternative would be to not try to be smart and always prompt twice. Note to :asuth -- If we decide that we want to keep this, currently that functionality is in `modifyAccount` in GELAM -- your call as to whether that would make more sense there or in Gaia. I added some tests for coverage and did some manual testing; once I hear back from Juwei about the questions above I'll do another round of testing and submit for review.
Oops, forgot to add needinfo. Juwei, could you take a look at my comment #16 above and let me know what you think about those UX questions?
(In reply to Marcus Cavanaugh [:mcav] from comment #16) > - There is now a password field directly below each of the "username" > fields for IMAP/POP and SMTP, in addition to the existing one at the > very top. This make three password fields total. My arm-chair UX suggestion would be that: - We keep all 3 fields - Include a checkbox labeled something like "Use the same password for sending and receiving" above the first password field. Check it by default. Only make it visible for IMAP and POP3. - When the checkbox gets unchecked we hide the first password box and show the other two password fields. - Use the existing fill/propagation logic you specify. Note that we will eventually be friendly and add a "Show password" checkbox beneath the password field in general. (Although it would be sorta-preferable if this was just something the platform addressed somehow.)
(In reply to Marcus Cavanaugh [:mcav] from comment #16) > 3) There's an important UX flow hidden in here: When you change your > password on the server, one or both of your account passwords could > be invalid. > I don't see a downside to this logic (as opposed to just always > prompting twice), but I also don't think iOS does this. The > alternative would be to not try to be smart and always prompt > twice. Note to :asuth -- If we decide that we want to keep this, > currently that functionality is in `modifyAccount` in GELAM -- your > call as to whether that would make more sense there or in Gaia. What you've implemented sounds right. In the common case, the user only updates the password once (like it is now) and everything still works. In the family of the uncommon case, we prompt a (necessary) second time, but help the user get it out of the way when they were already fixing their password situation. It's notable that what you implemented is better than Thunderbird's solution. In Thunderbird you currently would only be prompted to correct your SMTP password when you go to send an e-mail message. And Thunderbird would always prompt twice.
(In reply to Marcus Cavanaugh [:mcav] from comment #16) > 2) Secondly, that implementation raises questions about auto-filling > the fields in manual setup: I initially thought that if we do have > three password fields, that we should auto-fill the username and > password for the incoming and outgoing servers, however I now think > that's a very bad idea, because some servers require only the > pre-"@domain.com" part of the e-mail as the username, and others > want the whole thing. Without overcomplicating everything by trying > to autodetect the domain (which probably won't work since we're > already doing manual setup), auto-filling usernames seems like a > bad idea. However, right now the above logic also auto-fills > passwords -- that seems more likely to be reasonable, but I'm not > sure what's best. I think it's reasonable/friendly for us to auto-fill the entire e-mail address as the username. While I would not be surprised if there are servers out there that only want "user" and get angry if they see "user@domain", it's within the server's power to strip the "@domain" stuff and frequently will be harmless. For example, dovecot's authentication magic explicitly implements "username_format=%u" for when the auth file only includes "username" but the user authenticates using "user@domain". Since we also implement pretty "(x)" buttons to let the user clear input fields, some day we will probably implement selection mechanics, and it's arguably possible/easier for the user to position their cursor and delete the part they don't want in the jerky server case, I think we can make the mathematical argument for pre-filling/propagating from the e-mail address if we already have it. I do have a selfish motivation since it makes setting up my POP3 test account a lot easier, but I also consider myself a critically important portion of the userbase ;) Another thing we could also do is make manual configuration slightly smarter by trying dropping the "@domain" stuff if it's provided and the probing fails. I don't think that's something we would remotely want to do in this patch though.
Comment on attachment 8345559 [details] mail-fakeservers.html mail-fakeservers seems like it won't need further enhancements; r+-ing now, but feel free to wait until everything is totally done. Do clear if you change this non-trivially.
Attachment #8345559 - Flags: review+
Hi Marcus & Andrew!Thanks for all your advice! I'v installed the patch over my device, and it seems okay to have 3 password input fields in setting page. I also look to different devices as reference, some of them will auto-fill the username some will not. I think it would be not too harmful for users to refill the username if it's not the correct one. So let's auto-fill both the username & password ! For the possibility to bother users input password repeatedly, I got the same idea as what Andrew said: add a checkbox that labels "leverage same password for IMAP& SMTP (Default is checked. if users uncheck the box or case c) happen, lead users to prompt for SMTP password). Beside, I want to reorder the input field...it would be look better if 1. Host name first 2. User name second 3. Password third Any suggestions please let me know :)
Comment on attachment 8345556 [details] gelam.html Thanks for the feedback, Juwei and Andrew. I've updated the patches according to your comments. A few engineering notes for Andrew: - I removed the neckbeardy bitflagging in lieu of a simpler and more idiomatic structure. It was a bad idea. - I added a callback to modifyAccount and implemented a do_modifyAccount wrapper in th_main. I did not move any logic regarding clearAccountProblems per our IRC discussion. - I extracted the localization for setup_fix_password. - Gaia changes are in two more commits, one without GELAM, the last with. I'll be attaching a screenshot momentarily.
Attachment #8345556 - Flags: review?(bugmail)
Attachment #8345558 - Flags: review?(bugmail)
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
Thank you Marcus! It looks more organize now :) However I got only a small question: when the checkbox uncheck, I think the first password field should not disappear at the moment (Otherwise it would be a little bit wired that something missing). I suggest that to keep the first password field, but hide the IMAP password filed (since these two are the same, right?) Please correct me if anything I misunderstand...
(In reply to Juwei Huang from comment #27) > when the checkbox uncheck, I think the first password field should not > disappear at the moment (Otherwise it would be a little bit wired that > something missing). No, I think it is good as it is. Why should the password for the IMAP server not be in the section for the IMAP server? As I said it is good as it is, but another idea would be to put a checkbox in the section for the SMTP server to "Use IMAP credentials".
I did think it was slightly disorienting that the top password field disappeared. It causes you to lose your sense of place. (In my comments below, I'm saying IMAP where I mean IMAP/POP3.) What do you think about the following workflow: 1. We remove the "common" password field always, during manual setup, making the screen look something like iOS mail (attached). Also remove the checkbox option. 2. Before the user even enters manual setup, they're already presented with fields for username and password. I suggest that when we move to the manual setup screen, we copy whatever username/password they had there into both IMAP and SMTP fields. With that setup, if users had already entered their username and password before clicking "Manual Setup", we'd be able to prefill both IMAP and SMTP. Upsides: - Same as iOS mail (likely android too?) - Most ISPs list "custom e-mail client" setup in two separate sections, with information on IMAP credentials all in one group, and SMTP credentials in another group. Users might find it more straightforward to just enter their username/password twice if the ISP presents it that way. - Arguably clearer Downsides: - User may have to enter username/password twice, when in 99% of cases they're probably the same. An alternate-alternate suggestion -- we could do what I list above, but when the user enters their credentials in the IMAP section, we automatically prefill the SMTP section too. Then if they _are_ the same, they don't have to type them twice; if they aren't, they can just type over what's there. I don't feel too strongly about any one solution, but I do see how the checkbox can be a bit disorienting and confusing as it's currently presented. Juwei, thoughts?
Comment on attachment 8345556 [details] gelam.html Thanks for the separate commit! I made some comments directly on that commit. They show up in the pull request, but without context for me. I suggest clicking on the commit hash to view them in better context. r=asuth with requested changes made. (Note that the things I'm saying you don't need to fix, you don't need to fix. I know I sometimes have a very hard time deciding how to name things and end up making an arbitrary decision but would value feedback to my process to help me break ties in the future. So I'm doing that, but the specific cases don't need to be resolved.)
(In reply to Andrew Sutherland (:asuth) from comment #30) > r=asuth with requested changes made. (Probably already implied: And a clean travis run because travis is very unhappy because of the mail-fakeservers not yet being uploaded yet right now issue.)
Comment on attachment 8345558 [details] gaia.html Since you still have some suggestions for the manual setup UI and I'm lazy, I'm r=asuth'ing the parts here that don't have to do with that manual setup screen. But I'm downgrading it to a feedback+ to convey that this patch needs to ensure that it confirms with UX's call then needs further review. (We're not allowed to set open-ended review? flags anymore on components with listed reviewers, it appears.) I propose :jrburke review at that point because he is the decider for the front-end stuff and I don't think he's gotten a chance to weigh in on conditionalized UI idioms. Also, as a coincidental side-effect with practically no ulterior motives involved, then I don't have to review that bit.
Attachment #8345558 - Flags: review?(bugmail) → feedback+
> What do you think about the following workflow: > > 1. We remove the "common" password field always, during manual setup, > making the screen look something like iOS mail (attached). Also > remove the checkbox option. > > 2. Before the user even enters manual setup, they're already presented > with fields for username and password. I suggest that when we move > to the manual setup screen, we copy whatever username/password they > had there into both IMAP and SMTP fields. > > With that setup, if users had already entered their username and > password before clicking "Manual Setup", we'd be able to prefill both > IMAP and SMTP. > I totally agree with your concerns for comment #28 & comment #29, and I think it would be a good idea for comment #29 with some changes: 1. When initial Email app, 3 input field (username, mail, password) with a Manual Setup which is disable. 2. Once users fulfills their credentials, Manual Setup would be enable. Tap on Manual setup to go further. 3. In manual setup page, remove common password field & checkbox away, and prefill IMAP/SMTP password fields base on what users just entered in previous step. Users can refill SMAP if it's not the same as IMAP. I think this flow can somehow reduce the possibility of entering password twice and also achieve the purpose of entering credentials before go to manual setup page. Any thoughts? let me know :)
Needinfoing James Burke for his thoughts. -- James, some context: this bug allows the user to enter a different username and password for SMTP. We're discussing modifying the UX for manual account setup, with the goal to prefill as much as possible in the general case where both IMAP and SMTP credentials are identical. Presently, manual setup shows a common `password` field (along with `display name` and `email`). If the user has a different SMTP password, it's no longer accurate to have a "common" password field; there should really only be an IMAP password field and an SMTP password field. The bugzilla thread here outlines some of the UX discussion with Juwei. The latest proposal makes the following changes, diffed against the existing UI: 1. Disable the "manual setup" link until the user enters their username and password on the initial automatic setup screen. 2. On the manual setup screen, remove the "common" password field. Instead, place a password field in each of the IMAP/POP and SMTP sections. Step #1 is suggested so that we can prefill the most common case -- that both passwords are the same. Previous proposals in this thread included variants on updating the SMTP credentials when the IMAP credentials change, etc., with the potential confusion associated with us autofilling fields that the user didn't expect. Andrew suggested we get your thoughts on this, since you haven't had an opportunity to weigh in on this and you're well-versed in e-mail UX. He already reviewed the backend changes, and recommended that I flag you for review on the UI side once we've decided on the UX implementation.
Concern for me: if disabling the manual setup link, the user does not have enough knowledge to know what they need to do to enable it. They may try multiple taps to try to get it to activate, and be confused on why it does not? Another concern, something I had at first: when I very first used the email app a while back, the Next button was not immediately in my field of view when completing the form and I was unsure how to proceed for a moment. So, if a user *does not* want to do manual config, if they started typing, then see the manual config activate as clickable, then they may think that they need to click on that link to proceed to the next step, since the button in the header can be subtle. If that happens, they will likely get very confused. My first impulse on feedback: Enable manual config by default. When user goes to the manual config page, no common password field, just have it under each section. As user types in first password field, we auto-fill smtp name/password. If the form field values are different, then do not do the auto-update fill at that point. But I leave it to Juwei to judge the possible confusion of the non-clickable manual config link: if that seems manageable and not a concern, then proceed as you describe in your comment.
Flags: needinfo?(jrburke) → needinfo?(jhuang)
Created attachment 8358248 [details] New account_disable-enable.png I think what James concern is absolutely what ux try to avoid. Yet I still prefer to disable Manual Setup at the first time along with some visual changes to avoid confusion. Here's the proposal (please refer to attachment): We put "Next" & "Manual setup" buttons visually and functionally on the same level, which means both of the two buttons won't activate until credential areas have been filled. The benefit for this layout is that users can clearly know that they have to fill all the fields in order to do the next step no matter it is a manual or auto setup (erase the first concern), and also the "Next" button looks more obvious for user to proceed next step (erase the second concern). Any thoughts? Let me know :)
Juwei: would that also imply moving the Next button for the other parts of the account setup flow? There is one more Next button in the header for the autoconfig path (on the sync interval options card), and two for the manual path (one on the manual config card, and one on the sync interval options card).
Yes, I was thinking about that too. I think it's better to move all "next" button from header to the body list.
juawei: this is good, as long as there are ways to enter manual setup further along in the flow. After all, it might be common for a user to try the automatic setup, realise it doesn't work for them, and want to try specifying everything manually. Gerv
What's the l10n plan for those buttons? In your mock-up, "Manual Setup" (12 characters) is basically already at the limits of the button. The French translation is "Configuration manuelle" (22 characters). Check out http://transvision-beta.mozfr.org/?recherche=apps%2Femail%2Femail.properties%3Asetup-manual-config2&repo=gaia&sourcelocale=en-US&locale=fr&search_type=entities and play with 'target locale' to see other translations.
Andrew is right. How about change "Manual Setup" to "Advance" ?
(In reply to Juwei Huang from comment #41) > Andrew is right. How about change "Manual Setup" to "Advance" ? In English I think this only makes sense as "Advanced Setup". ("Advance" on its own would likely be interpreted as an awkward synonym for "next", which would really confuse people! :) I think most similar attempts to find single words may have similar problems because usually the word we'd use in English would be an adjective that makes little sense on its own. Ex: "Custom" is confusing compared to "Custom Setup" unless the buttons imply some similar concept. For example, if we had the buttons "Manual" and "Automatic", that leaves less question as to what the buttons do. Unfortunately I think it'd still be unclear which button the user wants to push. Maybe this is something we should try and sort out next week when we're all in the same place and have a whiteboard? :)
Created attachment 8369898 [details] new-manual-setup.mov I've attached a video of the new behavior as discussed in Taipei. (At least I think so, because I didn't write down exactly what we said.) Take a look and see if that matches with what you remember us talking about. Andrew, the code is also ready, but no need to review unless you think the UX is correct.  https://github.com/mcav/gaia/commit/8e53d89b2cf7e8cdff551abc15432e4cc42319d7
With a caveat that I probably broke some unit tests, but I'm waiting to fix those until I get word that the UX is right.
New flow looks good to me :-)) Will it pre-populate its best guess from the ISPDB anyway if you do manual setup, or not? Gerv
Comment on attachment 8369898 [details] new-manual-setup.mov It looks nice! thank you Marcus! The only thing I concern is the "Manual setup" button would be hided underneath the keyboard, the user might not discover it. Yet as we discussed before, the text string might not fit into the button if the button has not enough space. That's why we put the two buttons in separate lines. So my suggestion would be...that's stick the design in current stage, and let's see how's the feedback for it. And we probably would revise the button position based on the alignment of building block in the near future.
Attachment #8369898 - Flags: feedback?(jhuang) → feedback+
Target Milestone: 1.3 C1/1.4 S1(20dec) → 1.4 S1 (14feb)
Created attachment 8371769 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/14545 Andrew, looking for review/feedback on the Gaia changes. Gelam is unchanged save for rebasing onto latest master.
Whiteboard: [productivity-yvr-discuss] → [productivity-yvr-discuss][p=13]
(In reply to Juwei Huang from comment #46) > It looks nice! thank you Marcus! The only thing I concern is the "Manual > setup" button would be hided underneath the keyboard, the user might not > discover it. I know you're already fine with sticking with this for now, but I also think this actually works for our purposes. Specifically, the user who's not sure what to do would end up in this situation: - (The manual setup button is visible before the user clicks into any text fields so they know it's there.) - The keyboard covers the manual setup button. Our subtle scroll thumb shows up. - The user enters display name / e-mail / password - The user hits next - Autoconfig fails, we show an error message at the top of the page - *the keyboard is no longer visible* so the user can see the manual setup button - the user can now more easily click the manual setup button.
Attachment #8369898 - Flags: feedback?(bugmail) → feedback+
Comment on attachment 8371769 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/14545 looks great, works awesome. People are going to want to use the manual config instead of the autoconfig because it's so smoooooth. r=asuth conditional on fixing up the unit tests; I rather suspect you didn't update them while the actual UI strategy was up in the air :) What you're testing all seems good, though some can simply be discarded. I did try and help point out the moot logic/etc.
Attachment #8371769 - Flags: review?(bugmail) → review+
Target Milestone: 1.4 S1 (14feb) → 1.4 S2 (28feb)
Comment on attachment 8345556 [details] gelam.html Hey Andrew, I merged your fixes from bug 975298 (POP3 bad_password intermittent), but found that the changes I had made for this patch were somewhat obtuse and unclear. I've refactored test_account_bad_password_error to more concisely test all permutations of good/bad password combinations; in doing so, though, I had to change some of the logic you added in your POP3-intermittent patch slightly. I'd like you to take a look at the updated test and confirm that my changes don't inadvertently undo your previous fixes. I also made one small change to ActiveSync -- previously, it was reporting 'unknown' to checkAccount(err) when it really should just report 'bad-user-or-pass' in order to have the "should I report this to the user?" logic work correctly. There are a couple changes, specifically, that I'm unsure about: I took out the explicit call to checkAccount() because it seems redundant, given that clearProblems() also calls checkAccount() itself. Even so, there are two 'badlogin' events that get fired (see line 154 of the test); while I noted the reasons those events get fired in a comment, it seems strange to me that badlogin is reported twice, and it maybe seems like one of those should be squelched/not reported. I didn't touch any of the reporting logic there, so maybe that's actually expected. Lastly, IMAP/POP3 accounts also list 'connection' in account.problems -- it seems to me like this is also expected, according to the docs for account.problems, but the previous test's logic didn't encounter that. In any case, I think this prevents leaky events that get pushed to the next test step as we saw before, so my gut feeling is that this is what we should expect to see, but your eyes might catch something mine didn't. Your ArbPL changes to grey-out console.log() is nice, definitely helpful. I've only pushed the updated commit to GELAM, not Gaia yet.
Attachment #8345556 - Flags: review+ → review?(bugmail)
(In reply to Marcus Cavanaugh [:mcav] <firstname.lastname@example.org> from comment #50) > I also made one small change to ActiveSync -- previously, it was reporting > 'unknown' to checkAccount(err) when it really should just report > 'bad-user-or-pass' in order to have the "should I report this to the user?" > logic work correctly. Good improvement, thank you. ActiveSync account errors were applied in somewhat of a band-aid fashion and the side-effects of _reportErrorIfNecessary got us there. > There are a couple changes, specifically, that I'm unsure about: I took out > the explicit call to checkAccount() because it seems redundant, given that > clearProblems() also calls checkAccount() itself. Even so, there are two > 'badlogin' events that get fired (see line 154 of the test); while I noted > the reasons those events get fired in a comment, it seems strange to me that > badlogin is reported twice, and it maybe seems like one of those should be > squelched/not reported. I didn't touch any of the reporting logic there, so > maybe that's actually expected. Yeah, you are absolutely right to call this out, thanks for raising it, although you may be pulling your punches a little. I think we should do this: - Remove the explicit call to notifyBadLogin in mailbridge.js' _cmd_clearAccountProblems. AKA just remove that whole 'else' clause. - Bug 911937 is talking about changing/improving our bad password behaviour. We'll clean things up further there. This would mainly entail adding a callback to clearAccountProblems. The current explicit notifyBadLogin dumbness was roughly approximating that; there were also some shortcuts in UX behaviour that factored into that call. That should result in removing the second 'badLogin' expectation. > Lastly, IMAP/POP3 accounts also list > 'connection' in account.problems -- it seems to me like this is also > expected, according to the docs for account.problems, but the previous > test's logic didn't encounter that. Yeah, 'connection' is expected to show up, but it does not/should not trigger a badlogin notification itself since we filted in MailUniverse.__reportAccountProblem to only generate notifications for actionable/important errors. 'connection' is intended to let us convey that there is some potentially transient problem talking to the server. In an Andrew-designed UI, we might have a little yellow triangle warning sign on the account if the user went looking to see why stuff was not working great, but no need to bother them. If it's already a problem, it gets reported in the onbadlogin because we send the current state of the account over the wire, but otherwise it just gets reported via the __notifyModifiedAccount mechanism. > Your ArbPL changes to grey-out console.log() is nice, definitely helpful. To be clear, the grey background is for *any* log entry that happens outside of a test step. You may have gotten some other changes previously pushed to try and make some things less horrible to look at, though.
Which is to say, continued r=asuth for your changes, and as long as you're cool with the new change I propose and it doesn't blow up the tests, r=asuth for that too.
Rebased and landed, Travis is so happy. master: https://github.com/mozilla-b2g/gaia/commit/3250d4b48cc3b99a3e94baac879d00a27e8e9706 gelam: https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/1f106fea1b6810530c9eb35876616378dc4711b9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago → 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S4 (28mar)
I am happy as happy as Travis, since this is what I need for my mail setup. Can't wait to see 1.4. on my Geeksphone. Thanks guys!
[Environment] Gaia 9d0b1bdf746823a94b13e6574c1d8304dc584763 Gecko https://hg.mozilla.org/mozilla-central/rev/690c810c8e3e BuildID 20140409160202 Version 31.0a1 ro.build.version.incremental=eng.archermind.20131114.105818 ro.build.date=Thu Nov 14 10:58:33 CST 2013 [Result] PASS I mark it to verified.
Status: RESOLVED → VERIFIED
status-b2g-v1.4: --- → wontfix
status-b2g-v2.0: --- → fixed
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.