No error on duplicate calendar address, stalls

RESOLVED FIXED in 1.0b2

Status

Calendar
Calendar Views
--
minor
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Tyson Vanover, Assigned: Fallen)

Tracking

unspecified
1.0b2

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Thunderbird 2.0.0.16 (20080708), Lightning 0.8 (2008033120), Dgata 0.4

When adding a new network calendar, if you add the same address twice there is no error message.  Once you reach the screen where you enter the calendar name the next button activates but pushing it accomplishes nothing and there is no error warning you of what has gone wrong.

Reproducible: Always

Steps to Reproduce:
1.create new calendar
2.choose network calender
3.paste address of .xml calender and choose calender source (google in this case)
4.enter name of calender and assign color
5.repeat 1-4 again with the same calender address and source but name and color different
Actual Results:  
applications seems to become unresponsive

Expected Results:  
A popup warning that you already have added this calender and preferably what the local calendar is named.  Then redirection back to the page where the user provides the link.
(Assignee)

Comment 1

10 years ago
This is not a gdata-only bug, moving accordingly. I stumbled across this too, but since we are already in string freeze, we can't take this for the release. See related bug 314594, I thought there was also a general bug on error notifications in the new calendar wizard, but I couldn't find it.
Status: UNCONFIRMED → NEW
Component: Provider: GData → Calendar Views
Ever confirmed: true
OS: Windows Vista → All
QA Contact: gdata-provider → views
Hardware: PC → All

Comment 2

10 years ago
Did Bug 448206 improved situation in recent 0.9pre nightly builds?
(Assignee)

Comment 3

8 years ago
Created attachment 432573 [details] [diff] [review]
Fix - v1

This adds some simple error handling to the dialog. We might want to go for a more complete solution in a different bug, but this at least tells the user why the "Next" button is disabled.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #432573 - Flags: review?
(Assignee)

Comment 4

8 years ago
Created attachment 432575 [details]
Screenshot for ui-review

I'm happy to change colors if needed, but these use the system style for the <notification> element.
Attachment #432575 - Flags: ui-review?(clarkbw)
(Assignee)

Updated

8 years ago
Attachment #432573 - Flags: review? → review?(Mozilla)
Comment on attachment 432575 [details]
Screenshot for ui-review

I haven't tried this out yet, but does the "Please enter a valid location" appear while you are typing in a valid location?  I wouldn't think that's exactly ideal but it's simple and works.  Otherwise things look good.
Attachment #432575 - Flags: ui-review?(clarkbw) → ui-review+
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> (From update of attachment 432575 [details])
> I haven't tried this out yet, but does the "Please enter a valid location"
> appear while you are typing in a valid location?  I wouldn't think that's
> exactly ideal but it's simple and works.  Otherwise things look good.

Unfortunately, yes. I could postpone the checking a bit (i.e if no key was pressed for 500ms, then check, or on blur), that should work out.
(Assignee)

Comment 7

8 years ago
Created attachment 433788 [details] [diff] [review]
Fix - v2

This patch uses a search textbox to delay the checking for 500ms and also contains the file I forgot to hg add.
Attachment #432573 - Attachment is obsolete: true
Attachment #433788 - Flags: review?(Mozilla)
Attachment #432573 - Flags: review?(Mozilla)
Comment on attachment 433788 [details] [diff] [review]
Fix - v2

Patch looks good and works

One nit:
>+        let notificationbox = document.getElementById("location-notifications");
>         if (canAdvance && document.getElementById("calendar-uri").value &&
>             curPage.pageid == "locationPage") {
>-            canAdvance = checkURL();
>+            let [reason,] = parseUri(document.getElementById("calendar-uri").value);
>+            canAdvance = (reason == errorConstants.SUCCESS);
>+            setNotification(reason);
>+        } else {
Although indentation is correct here, it somewhat conceals the end of the if statement. Maybe you can indent the second line of it a little more.

Due to Bug 533220 it may be a good time to move these files to calendar/base/content/dialogs, since the code is cleaned up now.
If you prefer, I can prepare a seperate patch in a first step to fix Bug 533220.
This would include renaming to calendar-creation-wizard.xul and .js

With the current patch
r=markus
Attachment #433788 - Flags: review?(Mozilla) → review+
(Assignee)

Comment 9

8 years ago
(In reply to comment #8)
> Although indentation is correct here, it somewhat conceals the end of the if
> statement. Maybe you can indent the second line of it a little more.
Done

> Due to Bug 533220 it may be a good time to move these files to
> calendar/base/content/dialogs, since the code is cleaned up now.
> If you prefer, I can prepare a seperate patch in a first step to fix Bug
> 533220.
Yes, I'd prefer fixing this in the other bug. I wouldn't consider the code cleaned up there, but its good enough for a start :-)
(Assignee)

Comment 10

8 years ago
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/cb2333d71134>
-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
You need to log in before you can comment on or make changes to this bug.