impossible to view the list of attendees , an event a shared agenda with read access on it

RESOLVED FIXED in 4.7.2

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: informatique, Assigned: bv1578)

Tracking

({regression})

Lightning 4.7
4.7.2
regression

Details

Attachments

(7 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8750816 [details]
Capture.PNG

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20160420141331

Steps to reproduce:

1. create an event with several participating

2. consulted via the shared calendar caldav via another computer.
the user viewing the calendar is not one of the participants !

3. consult the event and see the list of participants is not present





Actual results:

the list of participants is not present in view event but if we consult source code participants are present


Expected results:

Users who consults shared calendars that are not participating in a meeting , can not see the list of participants.
(Reporter)

Comment 1

2 years ago
Created attachment 8750817 [details]
Capture1.PNG (not view participant)
(Reporter)

Comment 2

2 years ago
Created attachment 8750819 [details]
Capture2.PNG (code source of user consult shared agenda)

Comment 3

2 years ago
This is implemented this way since ages, but I don't know what was the motivation for this. Decathlon, do you happen to know why it wss implemented that way?
Status: UNCONFIRMED → NEW
Component: Calendar Views → Dialogs
Ever confirmed: true
Flags: needinfo?(bv1578)
(Assignee)

Comment 4

2 years ago
Created attachment 8751189 [details] [diff] [review]
patch-v1

I can reproduce this by setting the calendar to read-only (it reproduces screenshot attachment 8750817 [details]).

The box with id="item-attendees" stays hidden because window.attendees is undefined:
http://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-summary-dialog.js#318

This patch fixes this case and should be a general fix but it needs to be verified in the context reported in description to be sure that there isn't another issue, though in this case I can't verify.

Maybe could you, or the reporter, try the patch in a case like that?
Flags: needinfo?(bv1578)
Attachment #8751189 - Flags: review?(makemyday)
(Reporter)

Comment 5

2 years ago
for information I use the calendars to share with sogo .

after the behavior , it must be that the user has rights to write on the agenda or is part of the list of participants to view the list of participants
(Reporter)

Comment 6

2 years ago
in version 4.0.5.2 a user not present in the list of participating and having read permissions on a shared agenda could consult the list of participants ...
(Reporter)

Comment 7

2 years ago
I tested the patch but it does not work
(Reporter)

Comment 8

2 years ago
Created attachment 8751217 [details]
Capture4.PNG
(Reporter)

Comment 9

2 years ago
Created attachment 8751219 [details]
Capture5.PNG
(Reporter)

Comment 10

2 years ago
sorry , the patch works!
(Reporter)

Comment 11

2 years ago
Created attachment 8751233 [details]
Capture6.PNG
Attachment #8751219 - Attachment is obsolete: true
(Reporter)

Comment 12

2 years ago
this patch will be included you it in a future version ? can add an option (preference ) which allow or not the consultation of the list of participants on a calendar with read only ?
Assignee: nobody → bv1578
Status: NEW → ASSIGNED

Comment 13

2 years ago
A pref doesn't make a sense here, as we're in the realm of the user who displays the read-only calendar and not the owner of the calendar here. If I understand correctly, what you're asking for is an implementation of access control management for a remote calendar from within Lightning, to enable the calendar owner a fine graned control of what is published when sharing a calendar. This is a completely different thing then the subject of this bug. I think we have a bug for that already, but I currently cannot find it.

Comment 14

2 years ago
Comment on attachment 8751189 [details] [diff] [review]
patch-v1

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

I didn't recall we fall back to summary dialog in read-only case. I'm wondering why we have all the disable-on-readonly stuff in event dialog then, but that doesn't need to be discussed in this bug.

It looks like a regression of the recent changes to the dialog to me (so I probably didn't recall the previous behaviour correctly) - did this work in previous releases like 4.0 as expected? If it's a regression, we may want to consider an uplift to esr as it's a simple patch.

r+ for the patch provided a ui-r+ - the description and attendee boxes show up with white background while beeing read-only. Richard, is this expected or would we prefer a gray background for them to make it more obvious that this is not editable (just create an event with attendees in a calendar of choice, make the calendar read-only a open the event again)?
Attachment #8751189 - Flags: ui-review?(richard.marti)
Attachment #8751189 - Flags: review?(makemyday)
Attachment #8751189 - Flags: review+
Comment on attachment 8751189 [details] [diff] [review]
patch-v1

I give ui-r- because I think we should remove in read-only mode the white background and also the border to make it obvious. Without border and background both areas would then look like the header part in this dialog.
Attachment #8751189 - Flags: ui-review?(richard.marti) → ui-review-

Comment 16

2 years ago
Thanks, Richard. Removing the border as well is maybe something to reconsider. Imho at least the attendees need to be captured visually, otherwise this looks a little bit odd when resizing the dialog.
Created attachment 8752488 [details]
dialog_proposal.png

(In reply to MakeMyDay from comment #16)
> Thanks, Richard. Removing the border as well is maybe something to
> reconsider. Imho at least the attendees need to be captured visually,
> otherwise this looks a little bit odd when resizing the dialog.

This is how it could look. But yes, when the dialog is too big the attendees have too big gaps in between.
(Assignee)

Comment 18

2 years ago
(In reply to MakeMyDay from comment #14)

> I'm wondering why we have all the disable-on-readonly stuff in event dialog

I think they allow to disable the dialog when the status of a calendar changes while the dialog is open.
The list of calendars in the dropdown menu doesn't change (it lists all the writeable calendars in the moment the dialog was opened) but then, if you select a calendar that now is read-only, the dialog gets completely disabled. 
Maybe it would be useful if the dialog would get disabled even for the calendar currently selected otherwise saving the event generates an error, but this is another issue.

> It looks like a regression of the recent changes to the dialog to me (so I
> probably didn't recall the previous behaviour correctly) - did this work in
> previous releases like 4.0 as expected? If it's a regression, we may want to
> consider an uplift to esr as it's a simple patch.

I checked older version (TB 18 and 37) and the behavior was like that: the attendees list appears, with the old icons, inside a box with a white background.

> Richard, is this expected
> or would we prefer a gray background for them to make it more obvious that
> this is not editable (just create an event with attendees in a calendar of
> choice, make the calendar read-only a open the event again)?

I agree, the white background box gives the impression of an editable field and would be better without it but we should keep separated this issue in order to backport a fix for the original bug.


(In reply to Richard Marti (:Paenglab) from comment #17)

> This is how it could look. But yes, when the dialog is too big the attendees
> have too big gaps in between.

Are you working on a patch or it is just a mock-up?
(In reply to Decathlon from comment #18)
> (In reply to Richard Marti (:Paenglab) from comment #17)
> 
> > This is how it could look. But yes, when the dialog is too big the attendees
> > have too big gaps in between.
> 
> Are you working on a patch or it is just a mock-up?

It's only a mock-up. To work it would need a read-only attribute somewhere.

Comment 20

2 years ago
(In reply to Decathlon from comment #18)

> > It looks like a regression of the recent changes to the dialog to me (so I
> > probably didn't recall the previous behaviour correctly) - did this work in
> > previous releases like 4.0 as expected? If it's a regression, we may want to
> > consider an uplift to esr as it's a simple patch.
> 
> I checked older version (TB 18 and 37) and the behavior was like that: the
> attendees list appears, with the old icons, inside a box with a white
> background.
> 
> > Richard, is this expected
> > or would we prefer a gray background for them to make it more obvious that
> > this is not editable (just create an event with attendees in a calendar of
> > choice, make the calendar read-only a open the event again)?
> 
> I agree, the white background box gives the impression of an editable field
> and would be better without it but we should keep separated this issue in
> order to backport a fix for the original bug.

I agree to leave with the white background for this bug then, so that we can uplift it and leave it to a follow up bug to make it appear similar to what the suggested mockup.

If your quick with checkin and uplift, this still can go to 4.7.1.1, (I guess deadline for landings on ERS45 is tommorow for that), but 4.7.2 is probably ok as well. Do you also file the follow up bug?
Keywords: regression
(Assignee)

Comment 21

2 years ago
filed Bug 1272996 for the background of the attendees list.


Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/f09a0ae386a1
(Assignee)

Updated

2 years ago
Attachment #8751189 - Flags: approval-calendar-esr?(philipp)
Attachment #8751189 - Flags: approval-calendar-beta?(philipp)
Attachment #8751189 - Flags: approval-calendar-aurora?(philipp)
Target Milestone: --- → 5.1
Comment on attachment 8751189 [details] [diff] [review]
patch-v1

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

This patch seems simple enough, I'm ok taking this if it restores functionality. The patch has ui-r- though, it sounds like you want to make some further changes? Maybe it would make sense to do that in a separate bug.

Richard, is this patch ok as is for 4.7?
Attachment #8751189 - Flags: approval-calendar-esr?(philipp)
Attachment #8751189 - Flags: approval-calendar-esr+
Attachment #8751189 - Flags: approval-calendar-beta?(philipp)
Attachment #8751189 - Flags: approval-calendar-beta+
Attachment #8751189 - Flags: approval-calendar-aurora?(philipp)
Attachment #8751189 - Flags: approval-calendar-aurora+

Comment 23

2 years ago
The followup bug is mentioned in comment 21 already.
(Assignee)

Comment 24

2 years ago
(In reply to Philipp Kewisch [:Fallen] from comment #22)
> 
> This patch seems simple enough, I'm ok taking this if it restores
> functionality. The patch has ui-r- though, it sounds like you want to make
> some further changes? Maybe it would make sense to do that in a separate bug.

Yes, this is only for restoring functionality. Bug 1272996 is for changing the UI.


Pushed to:
https://hg.mozilla.org/releases/comm-aurora/rev/2efd26dbfe6b
https://hg.mozilla.org/releases/comm-beta/rev/55c4dc9f805f
https://hg.mozilla.org/releases/comm-esr45/rev/db466b11b439
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: 5.1 → 4.7.2
You need to log in before you can comment on or make changes to this bug.