Closed Bug 1583098 Opened 5 months ago Closed 5 months ago

set up reminder dialog - can't adjust the number of minutes/hours

Categories

(Calendar :: General, defect)

Lightning 68
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mkmelin, Assigned: pmorris)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

No description provided.

In the Set up Reminder dialog (From New event, go to Reminder | Custom...) , it's no longer possible to adjust the number of minutes/hours. You can adjust the number, but it will always just use 15 (the default) instead. You can still adjust the unit.

Summary: set up reminder dialog - can' → set up reminder dialog - can't adjust the number of minutes/hours

Works for me using Daily 71.0a1 (Build ID 20190920082822). When I set the reminder to e.g. 8 minutes before the value is correctly stored like

BEGIN:VALARM
ACTION:DISPLAY
TRIGGER:-PT8M

Tried trunk too. I'm talking about (at least) the values shown in the list when you try to add new "custom values". I.e. press Add, then adjust value, and above in the list, you get a new custom option. Those all get set to using 15 now

I see. It does work if I type in the new value using keyboard. But it does not work if I use the up/down arrow buttons to change the value.

Version: unspecified → Lightning 71

Thanks, Alice. So this is broken in TB 68, right?

Version: Lightning 71 → Lightning 7.0

I can reproduce on Tb68.1.0

Thanks for checking, much appreciated.

Well, looks like this was already reported 3 weeks ago with Bug 1578500.

Duplicate of this bug: 1578500

Well, looks like this was already reported 3 weeks ago with Bug 1578500.

We should establish a scheme to that bugs, particularly regressions, receive some attention even if they weren't filed internally by TB devs. I'm looking at all new non-Calendar bugs and NI'ing people to get regressions fixed.

This fixes clicking on the increment/decrement buttons in the hours/minutes number input in the custom reminder dialog.

Possibly related is bug 1556868, where this input was converted from a textbox to an html:input.

Assignee: nobody → paul
Status: NEW → ASSIGNED
Flags: needinfo?(paul)
Attachment #9094681 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9094681 [details] [diff] [review]
handle-reminder-number-input-clicks-0.patch

Let's not forget those requests in case the review passes.
Attachment #9094681 - Flags: approval-calendar-esr?(geoff)
Attachment #9094681 - Flags: approval-calendar-beta?(geoff)
Comment on attachment 9094681 [details] [diff] [review]
handle-reminder-number-input-clicks-0.patch

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

::: calendar/base/content/dialogs/calendar-event-dialog-reminder.xul
@@ +62,5 @@
>        <vbox id="reminder-relative-box" flex="1">
>          <hbox id="reminder-relative-length-unit-relation" flex="1">
>            <html:input id="reminder-length" type="number" min="0"
> +                      onkeyup="updateReminder(event)"
> +                      onclick="updateReminder(event)"/>

I'd think this should just be onchange 
Then onkeyup and onclick wouldn't be needed
Attachment #9094681 - Flags: review?(mkmelin+mozilla)

(In reply to Magnus Melin [:mkmelin] from comment #14)

I'd think this should just be onchange
Then onkeyup and onclick wouldn't be needed

Good thought. I just tried it and then entering numbers by keyboard doesn't work, so we still need onkeyup. I'd go with onkeyup and onclick so it's clear which actions are being handled.

Turns out that oninput does the job.

Attachment #9094681 - Attachment is obsolete: true
Attachment #9094681 - Flags: approval-calendar-esr?(geoff)
Attachment #9094681 - Flags: approval-calendar-beta?(geoff)
Attachment #9094695 - Flags: review?(mkmelin+mozilla)
Attachment #9094695 - Flags: approval-calendar-esr?(geoff)
Attachment #9094695 - Flags: approval-calendar-beta?(geoff)
Comment on attachment 9094695 [details] [diff] [review]
handle-reminder-number-input-clicks-1.patch

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

Oh right, thx! The way this dialog updates the item as you write is a bit unorthodox...
Attachment #9094695 - Flags: review?(mkmelin+mozilla) → review+
See Also: → 1583340
Target Milestone: --- → 71
Attachment #9094695 - Flags: approval-calendar-esr?(geoff)
Attachment #9094695 - Flags: approval-calendar-esr+
Attachment #9094695 - Flags: approval-calendar-beta?(geoff)
Attachment #9094695 - Flags: approval-calendar-beta+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/c4e121fd5c05
Handle clicks on reminder dialog number input. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Keywords: checkin-needed
Resolution: --- → FIXED

Works in 68.1.1 ESR.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.