632355 - calendar-summary-dialog : attendees names overlap

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[PM]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.2.13) Gecko/20101207 Lightning/1.0b3pre Thunderbird/3.1.7

Displaying an event containing labels of participants very long is not correct from a read-only calendar.
In this case, attendees names overlap. We need to resize the dialog to see correct display.

Reproducible: Always

Steps to Reproduce:
1.create un event and add multiples attendees with long names (more than 25 car) 
2.open it from a read-only calendar (share it or check read-only on the calendar)
3.calendar-summary-dialog open and we can see that attendees names overlap.


Expected Results:  
attendees names not overlapping at first display.

Comment 1

[PM]

Attachment: names overlapping

Comment 2

[PM]

Attachment: [after blockers]Patch v1 with some cosmetic improvement

Comment 3

[PM]

patch v1 display attendees in one column only, with sorted names.

Comment 4

[PM]

Attachment: displaying attendees with patch v1

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 510577 [details] [diff] [review]
[after blockers]Patch v1 with some cosmetic improvement

Please request review from me when you upload a patch. I'll have a look at this when the beta bugs are fixed, we have to give those bugs priority at the moment so we can proceed on the next release.

On a general note, please be sure to replace tabs with spaces, if you look at the patch as uploaded, you see the big difference in spacing :)

If I see things right, the downside to this patch is that only one row of attendees is shown, regardless of how much horizontal space is available. So if you have resized the dialog, we can't take advantage of it.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

Philippe, what do you think of this alternative? It keeps two columns and just crops the label.

This patch also hacks a fix for a XUL issue that the listbox columns don't contract once they have been made wider.

Comment 7

[PM]

(In reply to Philipp Kewisch [:Fallen] from comment #6)
> Created attachment 590694 [details] [diff] [review]
> Fix - v2
> 
> Philippe, what do you think of this alternative? It keeps two columns and
> just crops the label.
> 
> This patch also hacks a fix for a XUL issue that the listbox columns don't
> contract once they have been made wider.

I think the two solutions are good. 
we use one column because some user have a long display name (more than 50 chars).

Fix - v2:

if (attendee.commonName && attendee.commonName.length) {
  label.value = attendee.commonName;
  // XXX While this is correct from a XUL standpoint, it doesn't
  // seem to work on the listcell. Working around this would be an
  // evil hack, so I'm waiting for it to be fixed in the core
  // code instead.
  listcell.setAttribute("tooltiptext", attendee.toString());
  
=>  label is not defined (removed by the patch)

listcell.setAttribute("tooltiptext", attendee.toString());
=>
listcell.setAttribute("label",  attendee.commonName);
?

Comment 8

[PM]

(In reply to Philipp Kewisch [:Fallen] from comment #6)
> Created attachment 590694 [details] [diff] [review]
> Fix - v2
> 
> Philippe, what do you think of this alternative? It keeps two columns and
> just crops the label.
> 
> This patch also hacks a fix for a XUL issue that the listbox columns don't
> contract once they have been made wider.

with the v2 fix, if the event has three participants, a fourth icon is visible.

<listcell class="listcell-iconic status-icon" crop="end"/>
<listcell class="listcell-iconic status-icon" crop="end"/>
=>
<listcell class="listcell-iconic status-icon" crop="end" hidden="true"/>
<listcell class="listcell-iconic status-icon" crop="end" hidden="true"/>


listcell.setAttribute("status", attendee.participationStatus);
page++;
=>
listcell.setAttribute("status", attendee.participationStatus);
listcell.removeAttribute("hidden");
page++;

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 590694 [details] [diff] [review]
Fix - v2

I've fixed the issues you mentioned, thank you very much! I understand your reasoning with the one column, I think in the long run it should be fixed differently though:

It would be nice if the dialog could layout the attendees automatically. It would be cool if we could just add the attendees to a hbox and use something (maybe -moz-column-count and friends) to automatically sort them into as many columns as fit,  but some more logic might be needed to make sure it doesn't look strange if only one or two attendees have a very long name.

I recall working on a bug to fix that for the attendee tooltip some time back in 2007, but I resigned because at that point it was too complicated.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central changeset b0f2bbc5d752