Last Comment Bug 702613 - user is allowed to set invalid end time in events
: user is allowed to set invalid end time in events
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Dialogs (show other bugs)
: Lightning 1.1
: All All
: -- normal (vote)
: 1.9
Assigned To: Decathlon
:
Mentors:
Depends on:
Blocks: 702198
  Show dependency treegraph
 
Reported: 2011-11-15 07:01 PST by Hugo Miranda
Modified: 2013-02-07 07:53 PST (History)
3 users (show)
hmiranda: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (7.54 KB, patch)
2011-11-17 02:25 PST, Decathlon
philipp: review+
Details | Diff | Review
patch - v2 (3.10 KB, patch)
2012-01-23 01:41 PST, Decathlon
philipp: review+
Details | Diff | Review

Description Hugo Miranda 2011-11-15 07:01:17 PST
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 2011-11-17 02:25:53 PST
Created attachment 575126 [details] [diff] [review]
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] 2012-01-17 03:54:28 PST
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 2012-01-23 01:41:51 PST
Created attachment 590659 [details] [diff] [review]
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] 2012-01-23 04:58:54 PST
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] 2012-05-17 22:51:37 PDT
Decathlon, could you check my comment to see if we can push the patch?
Comment 6 Philipp Kewisch [:Fallen] 2012-08-09 11:36:15 PDT
Pushed to comm-central changeset 7ce307d297ff
Comment 7 Decathlon 2012-09-30 01:58:29 PDT
(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] 2013-02-07 07:53:30 PST
(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 :)

Note You need to log in before you can comment on or make changes to this bug.