702613 - user is allowed to set invalid end time in events

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[Hugo Miranda]

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.

Comment 1

[Decathlon]

Attachment: patch - v1

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.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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

Comment 3

[Decathlon]

Attachment: patch - v2

I apologize Philipp, but I attached a wrong patch :(
This is the right one.
Sorry for the waste of time.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Decathlon, could you check my comment to see if we can push the patch?

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central changeset 7ce307d297ff

Comment 7

[Decathlon]

(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.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(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 :)