Closed
Bug 759327
Opened 12 years ago
Closed 12 years ago
Account Provisioner search button should be disabled if the search input is empty
Categories
(Thunderbird :: Account Manager, defect)
Tracking
(thunderbird13+ fixed, thunderbird14+ fixed)
RESOLVED
FIXED
Thunderbird 15.0
People
(Reporter: mconley, Assigned: mconley)
References
Details
Attachments
(1 file)
14.03 KB,
patch
|
bwinton
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
STR: 1) Create a new profile, and wait for the Account Provisioner to open What happens? The search button is enabled, even though the search input is empty. What's expected? If the search input is empty, searching should not be possible.
Assignee | ||
Comment 1•12 years ago
|
||
So it turns out that this bug is a regression caused by bug 758618 (oh boy). This patch undoes bug 758618, and does things a bit better by using a built-in function that we should have been using instead. I've added a test, and modified our older tests to make sure that they're typing manually into the text input, as opposed to poking the value in with jQuery.
Assignee: nobody → mconley
Attachment #628028 -
Flags: review?(bwinton)
Assignee | ||
Updated•12 years ago
|
Blocks: 758618
Keywords: regression
Comment 2•12 years ago
|
||
Comment on attachment 628028 [details] [diff] [review] Patch v1 >+++ b/mail/components/newmailaccount/content/accountProvisioner.js >@@ -632,17 +629,17 @@ var EmailAccountProvisioner = { >- EmailAccountProvisioner.searchButtonEnabled(EmailAccountProvisioner._hasSupportedLang); >+ EmailAccountProvisioner.onSearchInputOrProvidersChanged(); That seems better… >+++ b/mail/test/mozmill/shared-modules/test-newmailaccount-helpers.js >@@ -86,19 +87,19 @@ function wait_for_provider_list_loaded(a > function wait_for_search_ready(aController) { > mc.waitFor(function() { > mc.sleep(0); >- return aController.window.$("#searchSubmit").is(":enabled"); >+ return aController.window.$("#name").is(":enabled"); Hmmm… Shouldn't those always be the same? And/or should we check both? Other than that, it seems reasonable. r=me.
Attachment #628028 -
Flags: review?(bwinton) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #2) > Comment on attachment 628028 [details] [diff] [review] > Patch v1 > > >+++ b/mail/components/newmailaccount/content/accountProvisioner.js > >@@ -632,17 +629,17 @@ var EmailAccountProvisioner = { > >- EmailAccountProvisioner.searchButtonEnabled(EmailAccountProvisioner._hasSupportedLang); > >+ EmailAccountProvisioner.onSearchInputOrProvidersChanged(); > > That seems better… > > >+++ b/mail/test/mozmill/shared-modules/test-newmailaccount-helpers.js > >@@ -86,19 +87,19 @@ function wait_for_provider_list_loaded(a > > function wait_for_search_ready(aController) { > > mc.waitFor(function() { > > mc.sleep(0); > >- return aController.window.$("#searchSubmit").is(":enabled"); > >+ return aController.window.$("#name").is(":enabled"); > > Hmmm… Shouldn't those always be the same? And/or should we check both? They aren't always the same, no. What wait_for_search_ready is for, is to cause us to spin until we're at the point where we can enter a search. Since we now do not enable the search button until there's text in the input, we need to key off of the input as opposed to the button. Which now makes me realize that bug 758618 really didn't cause this - I think this behaviour always existed, otherwise wait_for_search_ready would have always failed. Still, the new approach is better. > > Other than that, it seems reasonable. > > r=me. Thanks!
Assignee | ||
Updated•12 years ago
|
No longer blocks: 758618
tracking-thunderbird13:
--- → ?
tracking-thunderbird14:
--- → ?
Keywords: regression
Assignee | ||
Comment 4•12 years ago
|
||
Comment on attachment 628028 [details] [diff] [review] Patch v1 This one is not risky at all.
Attachment #628028 -
Flags: approval-comm-beta?
Attachment #628028 -
Flags: approval-comm-aurora?
Updated•12 years ago
|
Attachment #628028 -
Flags: approval-comm-beta?
Attachment #628028 -
Flags: approval-comm-beta+
Attachment #628028 -
Flags: approval-comm-aurora?
Attachment #628028 -
Flags: approval-comm-aurora+
Updated•12 years ago
|
Assignee | ||
Comment 5•12 years ago
|
||
comm-central: https://hg.mozilla.org/comm-central/rev/5d91dfc50d66 comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/5fdd178d8409 comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/0276e70d4f88
status-thunderbird13:
--- → fixed
status-thunderbird14:
--- → fixed
Target Milestone: --- → Thunderbird 15.0
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•