user is allowed to set invalid end time in events

RESOLVED FIXED in 1.9

Status

Calendar
Dialogs
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Hugo Miranda, Assigned: Decathlon)

Tracking

Lightning 1.1
Bug Flags:
in-litmus +

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.10 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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?
(Reporter)

Updated

6 years ago
Blocks: 702198
(Assignee)

Updated

6 years ago
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)

Updated

6 years ago
Assignee: nobody → bv1578
(Assignee)

Comment 1

6 years ago
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.
Attachment #575126 - Flags: review?(philipp)
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 3

6 years ago
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.
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.9
(Assignee)

Updated

5 years ago
Attachment #590659 - Flags: feedback?(bv1578)
(Assignee)

Comment 7

5 years ago
(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.