Closed Bug 1568434 Opened 5 years ago Closed 4 years ago

and pref calendar.itip.separateInvitationPerAttendee to determine whether the "Separate invitation per attendee" option is checked by default

Categories

(Calendar :: General, enhancement)

Lightning 6.2
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 72.0

People

(Reporter: black.fledermaus, Assigned: black.fledermaus)

Details

Attachments

(2 files, 3 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0 Waterfox/56.2.11

Steps to reproduce:

Just create a new Event in the calendar

Actual results:

"Notify attendees" is checked
"Separate invitation per attendee" is not checked

Expected results:

Find a way in Options-Menu or about:config to check "Separate invitation per attendee" by default

Attached patch lightning.js.diff (obsolete) β€” β€” Splinter Review
Attachment #9082406 - Flags: review+
Attached patch lightning-item-iframe.js.diff (obsolete) β€” β€” Splinter Review
Attachment #9082408 - Flags: review+

I have add some pachtes. I hope that will be accepted.

Comment on attachment 9082406 [details] [diff] [review]
lightning.js.diff

You really need to merge the two patches together and then we can find a reviewer for them.
Attachment #9082406 - Flags: review+
Attachment #9082408 - Flags: review+

And ideally you need to have a "proper" git diff, like here in attachment 9082410 [details] [diff] [review] or any other patch in any other bug you may find.

Attached patch bug1568434.diff (obsolete) β€” β€” Splinter Review
Attachment #9082406 - Attachment is obsolete: true
Attachment #9082408 - Attachment is obsolete: true
Attached patch bug1568434.diff β€” β€” Splinter Review
Attachment #9082837 - Attachment is obsolete: true

Hmm, that slipped through the cracks. Geoff, can you get this on the way?

Flags: needinfo?(geoff)

I'll accept adding a pref to allow the user to change the default, but I don't think we should change the default for all users. I'll review this code and see where we get to from here.

Flags: needinfo?(geoff)
Comment on attachment 9082838 [details] [diff] [review]
bug1568434.diff

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

Thank you for your patch. There's some changes needed. When you've done them, set the review flag to "?" and add my name in the box that appears.

::: calendar/lightning/content/lightning-item-iframe.js
@@ +776,4 @@
>                                         : (itemProp == "TRUE")));
>              let undiscloseProp = aItem.getProperty("X-MOZ-SEND-INVITATIONS-UNDISCLOSED");
>              undiscloseCheckbox.checked = (undiscloseProp === null)
> +                                         ? Preferences.get("calendar.itip.separateInvitationPerAttendee", true) // default value as most common within organizations

We don't use Preferences any more, use Services.prefs.getBoolPref with only the first argument. The comment can be removed.

@@ +855,5 @@
>  function changeUndiscloseCheckboxStatus() {
>      let notifyCheckbox = document.getElementById("notify-attendees-checkbox");
>      let undiscloseCheckbox = document.getElementById("undisclose-attendees-checkbox");
> +	if (!notifyCheckbox.checked) { undiscloseCheckbox.checked = false; }
> +		else { undiscloseCheckbox.checked = Preferences.get("calendar.itip.separateInvitationPerAttendee", true); }

This change is unnecessary. But if it wasn't, I'd be asking for a few code style changes, most obviously to use two spaces instead of tabs.
Attachment #9082838 - Flags: feedback+

Sorry, the patch does work anymore for me, even with the changes you suggest. I always get a "ReferenceError: Preferences is not defined" error. I don't know what else is wrong here. My skills are very, very basic. So please change this few lines by your own and push it to upstream.

I'am not able set the review flag

Congratulations on your first patch. I'll check it in at the next opportunity.

Assignee: nobody → black.fledermaus
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/06fd730429c0
Separate invitation per attendee, check by default; r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 72
Summary: Separate invitation per attendee, check by default → and pref calendar.itip.separateInvitationPerAttendee to determine whether the "Separate invitation per attendee" option is checked by default
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: