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)

x86
All
defect
Not set
normal

Tracking

(thunderbird13+ fixed, thunderbird14+ fixed)

RESOLVED FIXED
Thunderbird 15.0
Tracking Status
thunderbird13 + fixed
thunderbird14 + fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(1 file)

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.
Attached patch Patch v1Splinter Review
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)
Blocks: 758618
Keywords: regression
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+
(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!
No longer blocks: 758618
Keywords: regression
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?
Attachment #628028 - Flags: approval-comm-beta?
Attachment #628028 - Flags: approval-comm-beta+
Attachment #628028 - Flags: approval-comm-aurora?
Attachment #628028 - Flags: approval-comm-aurora+
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.

Attachment

General

Created:
Updated:
Size: