Closed Bug 702613 Opened 8 years ago Closed 7 years ago

user is allowed to set invalid end time in events

Categories

(Calendar :: Dialogs, defect)

Lightning 1.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hmiranda, Assigned: bv1578)

References

Details

Attachments

(1 file, 1 obsolete file)

When following Litmus prototype test 23: Negative Test - Event End before Start 

The user is able to set an end time before the start time by typing the time in the input field. If the next event is to press the Save button, the end time is automatically shifted to match the default event duration and the event is saved. In addition, a popup is shown warning the user about the wrong end time.
Flags: in-litmus+
Flags: blocking-calendar1.0?
Blocks: 702198
Component: General → Dialogs
Flags: blocking-calendar1.0?
OS: Linux → All
QA Contact: general → dialogs
Hardware: x86_64 → All
Version: Lightning 1.2 → Lightning 1.1
Assignee: nobody → bv1578
Attached patch patch - v1 (obsolete) — Splinter Review
This patch uses the same code I've added in the patch for bug 410427 about the until date. The global variable should allow to adapt the code for the wrong input of an end date and an until date as well.

If this patch will be reviewed before the patch for bug 410427, that patch will need an updating. Otherwise I can add an updated patch in bug 410427 which also includes this fix, just let me know what's better for you.
Attachment #575126 - Flags: review?(philipp)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
Comment on attachment 575126 [details] [diff] [review]
patch - v1

Review of attachment 575126 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, I reviewed the other patch too. r=philipp
Attachment #575126 - Flags: review?(philipp) → review+
Attached patch patch - v2Splinter Review
I apologize Philipp, but I attached a wrong patch :(
This is the right one.
Sorry for the waste of time.
Attachment #575126 - Attachment is obsolete: true
Attachment #590659 - Flags: review?(philipp)
Comment on attachment 590659 [details] [diff] [review]
patch - v2

Review of attachment 590659 [details] [diff] [review]:
-----------------------------------------------------------------

Don't worry, no harm done. Shame on me for not noticing, as I only reviewed the code :-)

In general r=philipp, I'd take this patch without changes if you like. See comment below and let me know if you'd like to change something or not.

::: calendar/base/content/dialogs/calendar-event-dialog.js
@@ +2341,5 @@
>      // confirmed (i.e. not followed by a click, a tab or enter keys pressure).
>      document.documentElement.focus();
>  
> +    // Don't save if a warning dialog about a wrong input date must be showed.
> +    if (gWarning) {

So I think if we just return here, clicking on save (without closing) with the warning being shown will just return, giving the user not much feedback that it didn't work.

Unless I missed this already happening, could you make the save button be disabled when the warning is shown? Also, we should think about what kind of feedback the user gets when he tries to close the dialog, gets the save/dont save/cancel dialog and then selects save. I assume the dialog just stays open then? I think ideally, the warning should flash somehow (either by making it go away and come back, or a real visual flash). But maybe that is too much for this bug.
Attachment #590659 - Flags: review?(philipp)
Attachment #590659 - Flags: review+
Attachment #590659 - Flags: feedback?(bv1578)
Decathlon, could you check my comment to see if we can push the patch?
Pushed to comm-central changeset 7ce307d297ff
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.9
Attachment #590659 - Flags: feedback?(bv1578)
(In reply to Philipp Kewisch [:Fallen] from comment #4)

> >      // confirmed (i.e. not followed by a click, a tab or enter keys pressure).
> >      document.documentElement.focus();
> >  
> > +    // Don't save if a warning dialog about a wrong input date must be showed.
> > +    if (gWarning) {
> 
> So I think if we just return here, clicking on save (without closing) with
> the warning being shown will just return, giving the user not much feedback
> that it didn't work.

In general when the warning dialog appears, the user can't do anything else than interact with it. Trying to click elsewhere (e.g. the save button on the main dialog), the warning dialog flashes.
 
> Unless I missed this already happening, could you make the save button be
> disabled when the warning is shown? Also, we should think about what kind of
> feedback the user gets when he tries to close the dialog, gets the save/dont
> save/cancel dialog and then selects save. I assume the dialog just stays
> open then? I think ideally, the warning should flash somehow (either by
> making it go away and come back, or a real visual flash). But maybe that is
> too much for this bug.

I will try to address these issues in the patch for bug 410427 that needs the same solution proposed here but for the "until" date.
(In reply to Decathlon from comment #7)
> > So I think if we just return here, clicking on save (without closing) with
> > the warning being shown will just return, giving the user not much feedback
> > that it didn't work.
> 
> In general when the warning dialog appears, the user can't do anything else
> than interact with it. Trying to click elsewhere (e.g. the save button on
> the main dialog), the warning dialog flashes.
Ah yes, sorry. I didn't have that in mind. Please ignore anything complicated that is of no use since the dialog is modal anyway :)
You need to log in before you can comment on or make changes to this bug.