1446748 - Convert the remaining non-photon design calendar icons

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[[:MakeMyDay]]

There are still some calendar icons that haven't been converted to photon design. We should still fix that for the upcoming ESR for a consistent look and feel.

These icons should at least be revisited and probably moved to the new design:

- the calendar/task view tab icons (open calendar and task view tab)

- the small icons that may appear next to the calendar list in calendar or task view: the lock if the calendar is read-only, the exclamation mark in the yellow triangle if the calendar is not available, the database icon if this is a cached calendar, the screen icon if this is a network calendar (the two latter ones are not displayed anymore in my current  local build, which would be a regression if true also for Daily)

- Task icon in calendar views (requires to have enabled the task in view feature in the menu: View->Calendar->Current view->Tasks in view and of cause a task dated for the current view)

- the calendar/task edit tab/dialog icons (create a new event and task in tab or dialog mode, which can be toogled in the calendar options -> general -> edit in tab mode)

- the icons in the invite attendees dialog for partstat, user type and partstat type (create a new event and click |invite attendees| icon in the toolbar

- the calendar icon in the imip bar displayed for received invitations (requires a received email invitation)

- the attendee icons in the invitation summary in received email invitations (requires a received email invitation)

The icons of the last and the third last should be harmonized if possible.

Comment 1

[Richard Marti (:Paenglab)]

For the tab icons we have already bug 1433776. The icons where made by ntim.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Kalender.svg

(In reply to [:MakeMyDay] from comment #0)
> 
> - the calendar icon in the imip bar displayed for received invitations
> (requires a received email invitation)

I made this icon some time ago. What do you think about using it in the IMIP bar? It's not Photon style but abstract enough to use as a icon that isn't a button.

Comment 3

[[:MakeMyDay]]

Comment on attachment 8959953 [details]
Kalender.svg

Thanks. This looks a little bit compressed to me. If possible, I'm in favour to adopt the photon style simply for consistency. Maybe we can recycle/adapt one of the facicons or the calendar icon from the mode toolbar.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: calendar-tab-icons.patch

This patch addresses point 1 and 4.

I'm using the same icons for the calendar- and task tab we use already for the buttons in the tab to recognize better they are the result after clicking them.

The event- and task dialog icons are new and slightly different to the other two icon to have a distinction between them and the main tabs.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: IMIP-icon.patch

This patch addresses point 6 and needs the first patch applied first.

Comment 6

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #0)
> 
> - Task icon in calendar views (requires to have enabled the task in view
> feature in the menu: View->Calendar->Current view->Tasks in view and of
> cause a task dated for the current view)

I tried this and saw in the code, it has different icons for start of task and end of task. I created a task over multiple day but I always get only the normal task icon (or the completed task icon) but newer the start or end icon. What am I doing wrong?

Comment 7

[[:MakeMyDay]]

Thanks for taking this. I haven't built with the patches yet, so I'll take care of the reviews later to also visually check them.

The imip icon is currently also used in the alarm popup window and the install.rdf - we should find a replacement for that, too.

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

> I tried this and saw in the code, it has different icons for start of task
> and end of task. I created a task over multiple day but I always get only
> the normal task icon (or the completed task icon) but newer the start or end
> icon. What am I doing wrong?

Create a task with a due date but without start date and vice versa to get displayed both icons.

For events spanning over serveral day with start and end time (not allday event), there are icons indicating the spanning, I didn't mention before.

And finally, I also didn't mention the bell icon used calendar view and event tab for reminders, but you're probably already aware of it.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: IMIP-icon.patch part 2

I added a 32px icon for the Add-on Manager and the alarm popup. For the IMIP bar I still use a 24px icon because I think the 32px we used until now is too big there.

The PNG icon is still used for the preferences window.

Comment 9

[[:MakeMyDay]]

Comment on attachment 8962018 [details] [diff] [review]
calendar-tab-icons.patch

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

Thanks for the patch. Reusing the mode toolbar icons for the view tabs is fine for me.

When applying the patch, I had some findings:

- the icons for item editing are only used in tab mode, in dialog mode the pre-existing icons are displayed (both in event dialog and summary dialog).
- the editing icons still look quite similar to the calendar/task view tab icons. Maybe we can visually overlay a pen icon, similar to the add event/task icons in the toolbar (and maybe with a glasses icon when using it for displying readonly events in the summary dialog).
- when starting TB not in full screeen mode, the mode toolbar is aligned right without an offset like in ESR52 - not sure whetherthis is related. However, placed there it's directly below/above the X icons for closing TB / the today pane, which seems a little unfortunate.

Comment 10

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #9)
> Comment on attachment 8962018 [details] [diff] [review]
> calendar-tab-icons.patch
> 
> Review of attachment 8962018 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the patch. Reusing the mode toolbar icons for the view tabs is
> fine for me.
> 
> When applying the patch, I had some findings:
> 
> - the icons for item editing are only used in tab mode, in dialog mode the
> pre-existing icons are displayed (both in event dialog and summary dialog).

This are ICOs on Windows and on Linux PNG. I'd need to convert them. But what colour should I use? On Windows 10 they are no more shown. The same on some Linux distros.

> - the editing icons still look quite similar to the calendar/task view tab
> icons. Maybe we can visually overlay a pen icon, similar to the add
> event/task icons in the toolbar (and maybe with a glasses icon when using it
> for displying readonly events in the summary dialog).

I can look into it.

> - when starting TB not in full screeen mode, the mode toolbar is aligned
> right without an offset like in ESR52 - not sure whetherthis is related.
> However, placed there it's directly below/above the X icons for closing TB /
> the today pane, which seems a little unfortunate.

With a late Daily? Then this would be bug 1446151. Maximise/normalise should fix it until next start.

Comment 11

[[:MakeMyDay]]

Comment on attachment 8962033 [details] [diff] [review]
IMIP-icon.patch part 2

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

Icons and sizing are looking good.

However, the icon from the install.rdf doesn't get picked up for me. In the addons-manager as well in the new addon-options menue entry in the hamburger button, the old application icon is displayed.

Beside that, what is the point not to replace the icons in preference window alongside with those in the in-content view?

Comment 12

[[:MakeMyDay]]

(In reply to Richard Marti (:Paenglab) from comment #10) 
> > - the icons for item editing are only used in tab mode, in dialog mode the
> > pre-existing icons are displayed (both in event dialog and summary dialog).
> 
> This are ICOs on Windows and on Linux PNG. I'd need to convert them. But
> what colour should I use? On Windows 10 they are no more shown. The same on
> some Linux distros.

What do you mean here - the icons are not displayed on Win10 (for me they do) or they shouldn't be displayed anymore according to Windows style guide?

> > - when starting TB not in full screeen mode, the mode toolbar is aligned
> > right without an offset like in ESR52 - not sure whetherthis is related.
> > However, placed there it's directly below/above the X icons for closing TB /
> > the today pane, which seems a little unfortunate.
> 
> With a late Daily? Then this would be bug 1446151. Maximise/normalise should
> fix it until next start.

Local build, pulled last weekend.

Comment 13

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #11)
> Comment on attachment 8962033 [details] [diff] [review]
> IMIP-icon.patch
> 
> Review of attachment 8962033 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Icons and sizing are looking good.
> 
> However, the icon from the install.rdf doesn't get picked up for me. In the
> addons-manager as well in the new addon-options menue entry in the hamburger
> button, the old application icon is displayed.

The last version this worked was from 26.3. After this date m-c began to remove the support for non-bootstrpped extensions. You could try to install your Lightning XPI in an older build to check.

> Beside that, what is the point not to replace the icons in preference window
> alongside with those in the in-content view?

I thought about it but never done it.

(In reply to [:MakeMyDay] from comment #12)
> (In reply to Richard Marti (:Paenglab) from comment #10) 
> > > - the icons for item editing are only used in tab mode, in dialog mode the
> > > pre-existing icons are displayed (both in event dialog and summary dialog).
> > 
> > This are ICOs on Windows and on Linux PNG. I'd need to convert them. But
> > what colour should I use? On Windows 10 they are no more shown. The same on
> > some Linux distros.
> 
> What do you mean here - the icons are not displayed on Win10 (for me they
> do) or they shouldn't be displayed anymore according to Windows style guide?

You mean the icons in the titlebar on the left?

> > > - when starting TB not in full screeen mode, the mode toolbar is aligned
> > > right without an offset like in ESR52 - not sure whetherthis is related.
> > > However, placed there it's directly below/above the X icons for closing TB /
> > > the today pane, which seems a little unfortunate.
> > 
> > With a late Daily? Then this would be bug 1446151. Maximise/normalise should
> > fix it until next start.
> 
> Local build, pulled last weekend.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: calendar-tab-icons.patch

How about this edit event/task icons?

Comment 15

[[:MakeMyDay]]

Comment on attachment 8964261 [details] [diff] [review]
calendar-tab-icons.patch

The icons look good in general - the pen could be a tad thinner, but it is also ok like it is.

Comment 16

[[:MakeMyDay]]

(In reply to Richard Marti (:Paenglab) from comment #13)
> (In reply to [:MakeMyDay] from comment #11)
> > Comment on attachment 8962033 [details] [diff] [review]
> > IMIP-icon.patch
> > 
> > Review of attachment 8962033 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Icons and sizing are looking good.
> > 
> > However, the icon from the install.rdf doesn't get picked up for me. In the
> > addons-manager as well in the new addon-options menue entry in the hamburger
> > button, the old application icon is displayed.
> 
> The last version this worked was from 26.3. After this date m-c began to
> remove the support for non-bootstrpped extensions. You could try to install
> your Lightning XPI in an older build to check.

I'm currently on a build of 24th or 24th of March that still build with calendar.

> > Beside that, what is the point not to replace the icons in preference window
> > alongside with those in the in-content view?
> 
> I thought about it but never done it.

Maybe also something for ESR60?
 
> > What do you mean here - the icons are not displayed on Win10 (for me they
> > do) or they shouldn't be displayed anymore according to Windows style guide?
> 
> You mean the icons in the titlebar on the left?

Yes, that ones -  I'll do the review once the handling for that is clear. Since the icons are currently displayed, we should either change them to photon or remove them all together if that is what OS style requires - what is the strategy for other parts of TB to that regard?

Comment 17

[Richard Marti (:Paenglab)]

Attachment: calendar-tab-icons.patch

I created some window icons. What do you think about them? Because the Photon icons are monochrome, I made them black on a white background. Only one colour on a transparent background doesn't work on all window colours.

The alarm dialog is missing. A monochrome bell wouldn't look so good and it wouldn't be clear what it is. We could use a clock instead, as the dialog is used to show on a defined time. What do you think?

Comment 18

[Richard Marti (:Paenglab)]

Attachment: calendar-tab-icons.patch

I've added now a bell icon for the alarm dialog.

Comment 19

[[:MakeMyDay]]

(In reply to Richard Marti (:Paenglab) from comment #17)
> Created attachment 8964401 [details] [diff] [review]
> calendar-tab-icons.patch
> 
> I created some window icons. What do you think about them? Because the
> Photon icons are monochrome, I made them black on a white background. Only
> one colour on a transparent background doesn't work on all window colours.

This looks ok for me - how does that play together with dark themes? 

> The alarm dialog is missing. A monochrome bell wouldn't look so good and it
> wouldn't be clear what it is. We could use a clock instead, as the dialog is
> used to show on a defined time. What do you think?

The colored bell looks somewhat odd to me. A monochrome should work, since we need it anyway also in event-tab/dialog to indicate a set reminder and the calendar views. But a clock would be ok either way.

Comment 20

[Richard Marti (:Paenglab)]

Attachment: calendar-tab-icons.patch part 1

Removed the yellow fill of the bell.

Comment 21

[Richard Marti (:Paenglab)]

Attachment: screenshot.png

This is how it looks with dark background.

Comment 22

[Richard Marti (:Paenglab)]

Attachment: calendar-alarm-icons.patch part 3

This changes the bell icons in the reminder definition dialog and in the views.

I tried to animate the icon when it's flashing but this didn't work, also when it works on other places. I now use a red background to highlight it.

The lock icon isn't implemented yet.

Comment 23

[Richard Marti (:Paenglab)]

Attachment: calendar-alarm-icons.patch part 3

I forgot to not package the no more used icons.

Comment 24

[Richard Marti (:Paenglab)]

Attachment: calendar-management.patch part 4

This patch exchanges the locked error icon in the calendar management tree.

Tree are special for SVG and do not support "fill" from CSS. I made the icons fixed yellow to be always visible. This, because Linux and Mac use a dark treeitem background when they are selected and the default black would make the icons almost invisible. Yellow is also good because this icons do inform that something isn't in normal state.

Comment 25

[[:MakeMyDay]]

This break the look & feel a bit, imho - to deal with the dark/light background issue, can't we just keep it monochrome and invert the icon using |filter: invert(100%)| if it's a dark theme?

Comment 26

[[:MakeMyDay]]

Comment on attachment 8966029 [details] [diff] [review]
calendar-alarm-icons.patch part 3

The bell icon looks good. Some finding with this patch

- if displayed in multi-week/monthly view and the event has a category assigned, the right most icon sits on top of the edge of the category color badge, which makes it hard to see correctly (may affect the privacy icon as well)

- if used as a window icon, the white background looks a litte odd (mostly visible in the flashing alarm, if your currently working in another application and hover over the flashing bar, but might alos be an issue if you use a Windows theme with a non-white dialog header) - is there a way to fill only the inner part of the icon (here the bell) and leave the outer part transparent (would apply to the other window icons as well)?

- I'm not sure whether the non-alarm and the email icons are still needed. Iirc, we don't support email alarms anymore.

Comment 27

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to [:MakeMyDay] from comment #26)
> - I'm not sure whether the non-alarm and the email icons are still needed.
> Iirc, we don't support email alarms anymore.

The gdata provider uses email alarms, not sure off hand if I repackaged the icon there or used the calendar css rules.

Comment 28

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #26)
> Comment on attachment 8966029 [details] [diff] [review]
> calendar-alarm-icons.patch part 3
> 
> The bell icon looks good. Some finding with this patch
> 
> - if displayed in multi-week/monthly view and the event has a category
> assigned, the right most icon sits on top of the edge of the category color
> badge, which makes it hard to see correctly (may affect the privacy icon as
> well)

This is a problem using a monochrome colour. I thought about a inverted border but the icon id too small to separate it well with the border.

> - if used as a window icon, the white background looks a litte odd (mostly
> visible in the flashing alarm, if your currently working in another
> application and hover over the flashing bar, but might alos be an issue if
> you use a Windows theme with a non-white dialog header) - is there a way to
> fill only the inner part of the icon (here the bell) and leave the outer
> part transparent (would apply to the other window icons as well)?

The colours aren't adaptive on ico and png. I can try with a reduced opacity of the white.

> - I'm not sure whether the non-alarm and the email icons are still needed.
> Iirc, we don't support email alarms anymore.

The email icon is used by the gdata provider with the calendar rules.

Comment 29

[[:MakeMyDay]]

(In reply to Richard Marti (:Paenglab) from comment #28)
> > - if displayed in multi-week/monthly view and the event has a category
> > assigned, the right most icon sits on top of the edge of the category color
> > badge, which makes it hard to see correctly (may affect the privacy icon as
> > well)
> 
> This is a problem using a monochrome colour. I thought about a inverted
> border but the icon id too small to separate it well with the border.

Can we just display the icons left (rsp. start-side) to the displayed category badge if any? It has been a poor visual experience before depending on the applied colors.

Comment 30

[Richard Marti (:Paenglab)]

Attachment: calendar-alarm-icons.patch part 3

(In reply to [:MakeMyDay] from comment #29)
> (In reply to Richard Marti (:Paenglab) from comment #28)
> > > - if displayed in multi-week/monthly view and the event has a category
> > > assigned, the right most icon sits on top of the edge of the category color
> > > badge, which makes it hard to see correctly (may affect the privacy icon as
> > > well)
> > 
> > This is a problem using a monochrome colour. I thought about a inverted
> > border but the icon id too small to separate it well with the border.
> 
> Can we just display the icons left (rsp. start-side) to the displayed
> category badge if any? It has been a poor visual experience before depending
> on the applied colors.

I made a different approach: When we have an alarm- or/and privacy icon, then don't paint the category colour to the top. With this the icons have still the event background.

If the event would be too small (less than ~16px tall) to show the category colour then it would be already now hard to spot the colour when the icons are above the category.

Comment 31

[Richard Marti (:Paenglab)]

Attachment: calendar-alarm-icons.patch part 3

The privacy icons are now SVGs too.

Comment 32

[Richard Marti (:Paenglab)]

Attachment: calendar-management.patch

Updated to apply after the changes in part 3.

Comment 33

[[:MakeMyDay]]

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

> I made a different approach: When we have an alarm- or/and privacy icon,
> then don't paint the category colour to the top. With this the icons have
> still the event background.

This only would help in day view and maybe in week view. In multi-week and month view, the items are only one liners so this doesn't resolve the issue.

> If the event would be too small (less than ~16px tall) to show the category
> colour then it would be already now hard to spot the colour when the icons
> are above the category.

Yes, the issue has been there before, but since we're touching this area now, we should fix that.

Comment 34

[[:MakeMyDay]]

Attachment: screenshot-win10-bug1446748-20180727.png

To pick up the review again, I applied the patches from this bug to the current tip pataches with fixes from bugs 1476947, 1477956, 1478578, 1478913, 1477478, 1477958 (in that order). The attached screenshot comprises almost all relevant icons and its occurrences which are subject to this bug just to make it easier when talking about it - the number refernces are also in the screenshot. So, let's get into the details:


A. Month/Multiweek View:

1. Tab icons for calendar view, tasks view, event editor and task editor are fine for me.
2. Not having icons for the editors in window mode is fine, too.
3. The only caveat is the windows default icon used for the grouped application windows when hoovering over the app icon in the Windows task bar. Probably a downside of not having definded icons for these windows, but nothing that need an urgent fix is there is no easy solution for it.
4. The calendar icon in the alarm dialog and the bell icon used in the event dialog and the calendar views are good as well.
5. Applying the alarm and privacy icons together with with the colored badge for categories is still an issue in these views. There is an overlapping that makes it hard to get it at a glance - I mentioned this already in one of the above comments. The badge should get its own dedicated space an make the other icon, if any, move a little to the left respective start side.
6. The task icons (completed and incomplete) are not yet converted
7. The indicators (in the calendar view) for event spanning on multiple days could deserve some adaptation, since the delicate appearance doesn't seem to fit in anymore.
8. The indicator for the same used in the today pane is not yet converted.


B. Day View:

1. Could you enlarge a little the vertical spacing between the icon and colored cataegory badge?
2. [not in the screenshot] The catagory badge now has also a gap if no icon is displayed. can you make that conditional?


C. Week View:
 
1. Similar issue like B. regarding the vertical space between the icon and the badge. Also, the text has no space to the icon (maybe also an issue in day view, but quite less likely to occur).
2. Not related, just something I noticed while checking this patches: tasks with a start time but no effective duration are cropped to much - but we can leave that for a follow-up bug (also not sure that this is an css issue).


D. Invitation Summary:

1. The calendar icon in the imip bar looks good.
2. The status/user-type/role icons may deserve some love, but probably they are ok for ESR60 - what do you think?
3. Just for completion. can you still check the icons used for the imip buttons whether all of them comply to the Photon design already? Iirc, you did already some chnages for them some months ago.


E. Calendar Widget:

1. Having these icons not monochrome would break the Photon approach a little. If the lock looks to massive in black, you maybe could add a transparant keyhole?
2. (not in the screenshot): we had also an icon to indicate offline support, which seems to be vanished - not sure whether it's just the icon or there is an issue with the code triggering it - can you at take care that we have a Photon-like icon for that?


F. Attendee Dialog, Event Summary Dialog, Event Dialog/Tab (not in the screenshot):

1. In the attendee dialog (see e.g. attachment 8402376 [details]), the role, user type and status icons are outdated. But we can leave them for now and should harmonize them with what we use when displying in the invitation summary (D), the event summary dialog and the event dialog/tab in the respective attendee section/tab.(I thought we had already a bug for it on file but cannot find it right now)
2. Updateting the icon for (D) would also be effective for the event summary dialog and the event dialog/tab since these are shared icons.

To summarize:

A.1 - A.4 and D.1 are good to go for me,
D.2, D3 just a need a cross check,
A.6 to A.8 (and maybe E.1 / E.2) are still needing (imporoved) icons and
A5, B1, B.2, C.1 need some tweaks

I suggest to leave out F.1, F2. and C.2  for now and follow-up later.

If we still can get something ready for the ESR build, this would be great, but since we're quite late in the cycle, we also can ship it with the next available dot release when we're done here. If you have no time to work on this atm, please leave a comment so we know whether there is still something to consider for 60.0 or not.

Comment 35

[[:MakeMyDay]]

Attachment: screenshot-win10-bug1446748-20180727-2.png

This version now has also the mentioned references in it.

Comment 36

[Richard Marti (:Paenglab)]

Attachment: Part 1: calendar-tab-icons.patch

(In reply to [:MakeMyDay] from comment #34)
> 
> A. Month/Multiweek View:
> 
> 1. Tab icons for calendar view, tasks view, event editor and task editor are
> fine for me.
> 2. Not having icons for the editors in window mode is fine, too.
> 3. The only caveat is the windows default icon used for the grouped
> application windows when hoovering over the app icon in the Windows task
> bar. Probably a downside of not having definded icons for these windows, but
> nothing that need an urgent fix is there is no easy solution for it.
> 4. The calendar icon in the alarm dialog and the bell icon used in the event
> dialog and the calendar views are good as well.
> 5. Applying the alarm and privacy icons together with with the colored badge
> for categories is still an issue in these views. There is an overlapping
> that makes it hard to get it at a glance - I mentioned this already in one
> of the above comments. The badge should get its own dedicated space an make
> the other icon, if any, move a little to the left respective start side.
> 6. The task icons (completed and incomplete) are not yet converted
> 7. The indicators (in the calendar view) for event spanning on multiple days
> could deserve some adaptation, since the delicate appearance doesn't seem to
> fit in anymore.
> 8. The indicator for the same used in the today pane is not yet converted.
> 
> 
> B. Day View:
> 
> 1. Could you enlarge a little the vertical spacing between the icon and
> colored cataegory badge?
> 2. [not in the screenshot] The catagory badge now has also a gap if no icon
> is displayed. can you make that conditional?
> 
> 
> C. Week View:
>  
> 1. Similar issue like B. regarding the vertical space between the icon and
> the badge. Also, the text has no space to the icon (maybe also an issue in
> day view, but quite less likely to occur).
> 2. Not related, just something I noticed while checking this patches: tasks
> with a start time but no effective duration are cropped to much - but we can
> leave that for a follow-up bug (also not sure that this is an css issue).
> 
> 
> D. Invitation Summary:
> 
> 1. The calendar icon in the imip bar looks good.
> 2. The status/user-type/role icons may deserve some love, but probably they
> are ok for ESR60 - what do you think?
> 3. Just for completion. can you still check the icons used for the imip
> buttons whether all of them comply to the Photon design already? Iirc, you
> did already some chnages for them some months ago.
> 
> 
> E. Calendar Widget:
> 
> 1. Having these icons not monochrome would break the Photon approach a
> little. If the lock looks to massive in black, you maybe could add a
> transparant keyhole?
> 2. (not in the screenshot): we had also an icon to indicate offline support,
> which seems to be vanished - not sure whether it's just the icon or there is
> an issue with the code triggering it - can you at take care that we have a
> Photon-like icon for that?
> 
> 
> F. Attendee Dialog, Event Summary Dialog, Event Dialog/Tab (not in the
> screenshot):
> 
> 1. In the attendee dialog (see e.g. attachment 8402376 [details]), the role,
> user type and status icons are outdated. But we can leave them for now and
> should harmonize them with what we use when displying in the invitation
> summary (D), the event summary dialog and the event dialog/tab in the
> respective attendee section/tab.(I thought we had already a bug for it on
> file but cannot find it right now)
> 2. Updateting the icon for (D) would also be effective for the event summary
> dialog and the event dialog/tab since these are shared icons.
> 
> To summarize:
> 
> A.1 - A.4 and D.1 are good to go for me,

OK

> D.2, D3 just a need a cross check,

D.2: Yes, this needs to be looked in a follow-up bug.
D.3: They where exchanged during the Photon icon change. SO they are Photon proof.

> A.6 to A.8 (and maybe E.1 / E.2) are still needing (imporoved) icons and
> A5, B1, B.2, C.1 need some tweaks

A.6 - A.8: Added new icons (A.7 wasn't done using icons but Unicode letters).
A5, B1, B.2, C.1: I moved the icons to the left to let the badge use the whole height. The problem with the icons above the badge is, when the event is very short, the badge can be very small below the icons and thus not visible. This can make C.1 with the text having no space a bit more visible. But this problem  is a issue because the text and the icons are in different layers and so it's hard to fix it, except with a rework of the event box.

> I suggest to leave out F.1, F2. and C.2  for now and follow-up later.

I think, this should be ready for c-c. For 60 I then need to look what needs to be rebased.

Comment 37

[Richard Marti (:Paenglab)]

Attachment: Part 2: IMIP-icon.patch

No change here but uploading again to be in correct patch order.

Comment 38

[Richard Marti (:Paenglab)]

Attachment: Part 3: calendar-today.patch

New part for the today pane multiday span arrows.

Comment 39

[Richard Marti (:Paenglab)]

Attachment: Part 4: calendar-alarm-icons.patch

Comment 40

[Richard Marti (:Paenglab)]

Attachment: Part 5: calendar-management.patch

Comment 41

[Richard Marti (:Paenglab)]

Comment on attachment 8995798 [details] [diff] [review]
Part 1: calendar-tab-icons.patch

I forgot to mention. The ICO files in the extensions chrome directory don't apply to show as window icons in the task bar. I moved them now to the TB's ICO to be at the same place. If SM agrees, I could do the same. But maybe they would like to have the old icons.

Comment 42

[[:MakeMyDay]]

Attachment: calendar-photon-take2.png

Thank you for the updates. We're getting closer - the attached screenshot illustrates the below mentioned issues (win10 again):

1. The icon for unavailable calendars is cut off at the right/end side a little. And it looks a ted bigger then the lock item.

2. The icon for completed/not completed tasks are ok, but maybe it's hard to say which is for what purpose (if you don't know it already, since both have only checkmarks indicating a completion in it but none has an indicator for a not completed task.

3. The arrowsheads used for the icons for events spanning multiple days seem to have different sizes. Only the icon for the ending of the span looks sharp, the other two icons look a lttle blury. Are these svg icons?

4. In week view the icons and the category badget doesn't seem to have a dedicated space. This is fine already in the other views.

5. In week view, long summries are not cropped unlike it is the case in the other views.

Comment 43

[Richard Marti (:Paenglab)]

Attachment: Part 3: calendar-today.patch

(In reply to [:MakeMyDay] from comment #42)
> Created attachment 8995902 [details]
> calendar-photon-take2.png
> 
> 1. The icon for unavailable calendars is cut off at the right/end side a
> little. And it looks a ted bigger then the lock item.

Fixed, it's smaller now.

> 2. The icon for completed/not completed tasks are ok, but maybe it's hard to
> say which is for what purpose (if you don't know it already, since both have
> only checkmarks indicating a completion in it but none has an indicator for
> a not completed task.

I changed the not completed icon from using checkmarks to use dots.

> 3. The arrowsheads used for the icons for events spanning multiple days seem
> to have different sizes. Only the icon for the ending of the span looks
> sharp, the other two icons look a lttle blury. Are these svg icons?

Fixed.

> 4. In week view the icons and the category badget doesn't seem to have a
> dedicated space. This is fine already in the other views.

This isn't the focus of this bug. But I played a bit and this should be fixed now. The badge is now the only child of the stack. Moving it out of it made it shrink. I tried a lot of other solution but failed. If you don't like it, I can revert it and let it for a other bug.

> 5. In week view, long summries are not cropped unlike it is the case in the
> other views.

Again, this is not the focus of this bug and would probably need some changes in the event box model -> new bug.

PS: Have you noticed in the screenshot, that the day Tab is active but the multiweek is shown. Never seen this here.

Comment 44

[Richard Marti (:Paenglab)]

Attachment: Part 4: calendar-alarm-icons.patch

Comment 45

[Richard Marti (:Paenglab)]

Attachment: Part 5: calendar-management.patch

Comment 46

[[:MakeMyDay]]

Thanks for the update.

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

> > 1. The icon for unavailable calendars is cut off at the right/end side a
> > little. And it looks a ted bigger then the lock item.
> 
> Fixed, it's smaller now.

This remove the dot of the exclamation mark in the triangle for me.

> > 2. The icon for completed/not completed tasks are ok, but maybe it's hard to
> > say which is for what purpose (if you don't know it already, since both have
> > only checkmarks indicating a completion in it but none has an indicator for
> > a not completed task.
> 
> I changed the not completed icon from using checkmarks to use dots.

Looks good.

 
> > 3. The arrowsheads used for the icons for events spanning multiple days seem
> > to have different sizes. Only the icon for the ending of the span looks
> > sharp, the other two icons look a lttle blury. Are these svg icons?
> 
> Fixed.

I cannot see any change here - still a little blury and different sizes of the arrowheads in the view (not the today pane).

> 
> > 4. In week view the icons and the category badget doesn't seem to have a
> > dedicated space. This is fine already in the other views.
> 
> This isn't the focus of this bug. But I played a bit and this should be
> fixed now. The badge is now the only child of the stack. Moving it out of it
> made it shrink. I tried a lot of other solution but failed. If you don't
> like it, I can revert it and let it for a other bug.
> 
> > 5. In week view, long summries are not cropped unlike it is the case in the
> > other views.
> 
> Again, this is not the focus of this bug and would probably need some
> changes in the event box model -> new bug.

Thanks, these are fine for now.

> PS: Have you noticed in the screenshot, that the day Tab is active but the
> multiweek is shown. Never seen this here.

I thought you would recognize the view regardless ;-) That's an effect I haven't fully understand yet and I think is releated to the new loading mechanism. If there are JS errors at other places while loading, I observed that. This still needs further investigation.

Comment 47

[[:MakeMyDay]]

Comment on attachment 8995799 [details] [diff] [review]
Part 2: IMIP-icon.patch

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

r+ with comment considered.

For the esr backport, we will need a version of thes patch that deals with install.rdf instead of manifest.json

::: calendar/base/jar.mn
@@ +125,5 @@
>      ../skin/common/icons/edit.svg                     (themes/common/icons/edit.svg)
>      ../skin/common/icons/event.svg                    (themes/common/icons/event.svg)
>      ../skin/common/icons/find.svg                     (themes/common/icons/find.svg)
>      ../skin/common/icons/freebusy.svg                 (themes/common/icons/freebusy.svg)
> +    ../skin/common/icons/icon32.svg                   (themes/common/icons/icon32.svg)

Plaese also remeove the previous cal-icon32.png files in the jar.mn and the tree.

Comment 48

[[:MakeMyDay]]

Comment on attachment 8995798 [details] [diff] [review]
Part 1: calendar-tab-icons.patch

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

r+ with comments considered.

::: calendar/base/jar.mn
@@ +156,5 @@
>      ../skin/common/ok-cancel.png                      (themes/common/images/ok-cancel.png)
>      ../skin/common/task-images.png                    (themes/common/images/task-images.png)
>      ../skin/common/timezone_map.png                   (themes/common/images/timezone_map.png)
>      ../skin/common/timezones.png                      (themes/common/images/timezones.png)
>      ../skin/common/calendar-event-dialog.png          (themes/common/dialogs/images/calendar-event-dialog.png)

Do we still need this?

::: calendar/lightning/jar.mn
@@ -81,5 @@
>  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning/skin/lightning-toolbar.css
>  % style chrome://calendar/content/calendar-event-dialog.xul chrome://communicator/skin/communicator.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
>  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning-common/skin/lightning.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
>  % style chrome://messenger/content/customizeToolbar.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> -    ../skin/lightning-common/mode-switch-icons.png            (themes/common/images/mode-switch-icons.png)

The seems not to be removed with this patch. Is an icon from this set still used elsewhere?

@@ -105,5 @@
>      ../skin/windows/lightning/imip.css                        (themes/windows/imip.css)
>      ../skin/windows/lightning/lightning.css                   (themes/windows/lightning.css)
>      ../skin/windows/lightning/lightning-toolbar.css           (themes/windows/lightning-toolbar.css)
>      ../skin/windows/lightning/lightning-widgets.css           (themes/windows/lightning-widgets.css)
> -    ../skin/windows/lightning/mode-switch-icons-aero.png      (themes/windows/images/mode-switch-icons-aero.png)

Same here

Comment 49

[Richard Marti (:Paenglab)]

Attachment: eventSpan.png

(In reply to [:MakeMyDay] from comment #46)
>  
> > > 3. The arrowsheads used for the icons for events spanning multiple days seem
> > > to have different sizes. Only the icon for the ending of the span looks
> > > sharp, the other two icons look a lttle blury. Are these svg icons?
> > 
> > Fixed.
> 
> I cannot see any change here - still a little blury and different sizes of
> the arrowheads in the view (not the today pane).

This is how the arrows look on a 100% desktop. I see them normally also a bit blurry because my desktop is scaled. It seems Windows draws all in 100% and scales then the result to the configured scale.

Do you use a scaled desktop too? If yes, could you try it with 100% scale?

Comment 50

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #46)
>  
> > > 3. The arrowsheads used for the icons for events spanning multiple days seem
> > > to have different sizes. Only the icon for the ending of the span looks
> > > sharp, the other two icons look a lttle blury. Are these svg icons?
> > 
> > Fixed.
> 
> I cannot see any change here - still a little blury and different sizes of
> the arrowheads in the view (not the today pane).

(In reply to [:MakeMyDay] from comment #48)
> Comment on attachment 8995798 [details] [diff] [review]
> Part 1: calendar-tab-icons.patch
> 
> Review of attachment 8995798 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with comments considered.
> 
> ::: calendar/base/jar.mn
> @@ +156,5 @@
> >      ../skin/common/ok-cancel.png                      (themes/common/images/ok-cancel.png)
> >      ../skin/common/task-images.png                    (themes/common/images/task-images.png)
> >      ../skin/common/timezone_map.png                   (themes/common/images/timezone_map.png)
> >      ../skin/common/timezones.png                      (themes/common/images/timezones.png)
> >      ../skin/common/calendar-event-dialog.png          (themes/common/dialogs/images/calendar-event-dialog.png)
> 
> Do we still need this?

Yes, for the priority in the event's status bar. I can look to do this through a SVG in the new part 6, okay?

> ::: calendar/lightning/jar.mn
> @@ -81,5 @@
> >  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning/skin/lightning-toolbar.css
> >  % style chrome://calendar/content/calendar-event-dialog.xul chrome://communicator/skin/communicator.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> >  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning-common/skin/lightning.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> >  % style chrome://messenger/content/customizeToolbar.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> > -    ../skin/lightning-common/mode-switch-icons.png            (themes/common/images/mode-switch-icons.png)
> 
> The seems not to be removed with this patch. Is an icon from this set still
> used elsewhere?

The icon itself isn't removed in the tree but not packaged. When I tried to remove a icon some time ago, Philipp didn't want I remove it from the tree. Since then I leave them.

> @@ -105,5 @@
> >      ../skin/windows/lightning/imip.css                        (themes/windows/imip.css)
> >      ../skin/windows/lightning/lightning.css                   (themes/windows/lightning.css)
> >      ../skin/windows/lightning/lightning-toolbar.css           (themes/windows/lightning-toolbar.css)
> >      ../skin/windows/lightning/lightning-widgets.css           (themes/windows/lightning-widgets.css)
> > -    ../skin/windows/lightning/mode-switch-icons-aero.png      (themes/windows/images/mode-switch-icons-aero.png)
> 
> Same here

Same here :-)

Comment 51

[[:MakeMyDay]]

(In reply to Richard Marti (:Paenglab) from comment #49)
 
> Do you use a scaled desktop too? If yes, could you try it with 100% scale?

Thanks, this was it - when setting to 100% everything is quite sharp.

(In reply to Richard Marti (:Paenglab) from comment #50)
> > >      ../skin/common/calendar-event-dialog.png          (themes/common/dialogs/images/calendar-event-dialog.png)
> > 
> > Do we still need this?
> 
> Yes, for the priority in the event's status bar. I can look to do this
> through a SVG in the new part 6, okay?

Yes, that would be great.

> > ::: calendar/lightning/jar.mn
> > @@ -81,5 @@
> > >  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning/skin/lightning-toolbar.css
> > >  % style chrome://calendar/content/calendar-event-dialog.xul chrome://communicator/skin/communicator.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> > >  % style chrome://messenger/content/customizeToolbar.xul chrome://lightning-common/skin/lightning.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> > >  % style chrome://messenger/content/customizeToolbar.xul chrome://calendar-common/skin/dialogs/calendar-event-dialog.css application={92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}
> > > -    ../skin/lightning-common/mode-switch-icons.png            (themes/common/images/mode-switch-icons.png)
> > 
> > The seems not to be removed with this patch. Is an icon from this set still
> > used elsewhere?
> 
> The icon itself isn't removed in the tree but not packaged. When I tried to
> remove a icon some time ago, Philipp didn't want I remove it from the tree.
> Since then I leave them.
> 
> > @@ -105,5 @@
> > >      ../skin/windows/lightning/imip.css                        (themes/windows/imip.css)
> > >      ../skin/windows/lightning/lightning.css                   (themes/windows/lightning.css)
> > >      ../skin/windows/lightning/lightning-toolbar.css           (themes/windows/lightning-toolbar.css)
> > >      ../skin/windows/lightning/lightning-widgets.css           (themes/windows/lightning-widgets.css)
> > > -    ../skin/windows/lightning/mode-switch-icons-aero.png      (themes/windows/images/mode-switch-icons-aero.png)
> > 
> > Same here
> 
> Same here :-)

Philipp, is there still any reason why we should keep these not longer used png icons in the tree?

Comment 52

[Richard Marti (:Paenglab)]

Attachment: Part 2: IMIP-icon.patch

(In reply to [:MakeMyDay] from comment #47)
> Comment on attachment 8995799 [details] [diff] [review]
> Part 2: IMIP-icon.patch
> 
> Review of attachment 8995799 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with comment considered.
> 
> For the esr backport, we will need a version of thes patch that deals with
> install.rdf instead of manifest.json
> 
> ::: calendar/base/jar.mn
> @@ +125,5 @@
> >      ../skin/common/icons/edit.svg                     (themes/common/icons/edit.svg)
> >      ../skin/common/icons/event.svg                    (themes/common/icons/event.svg)
> >      ../skin/common/icons/find.svg                     (themes/common/icons/find.svg)
> >      ../skin/common/icons/freebusy.svg                 (themes/common/icons/freebusy.svg)
> > +    ../skin/common/icons/icon32.svg                   (themes/common/icons/icon32.svg)
> 
> Plaese also remeove the previous cal-icon32.png files in the jar.mn and the
> tree.

Mac uses the cal-icon32.png still for the account central. And Linux and Windows the cal-icon24.png for it. Changing it now would make the calendar icon look like a alien with the other icons. This should be changed together with the other icons for the account central (no open bug).

I removed now the not used icons from the packaging.

Comment 53

[Richard Marti (:Paenglab)]

Attachment: Part 5: calendar-management.patch

Updated after the part 2 changes.

Comment 54

[Richard Marti (:Paenglab)]

Attachment: Part 6: calendar-statusbar-priority.patch

This uses now a SVG for the event's status bar priority icons.

Comment 55

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to [:MakeMyDay] from comment #51)
> > > The seems not to be removed with this patch. Is an icon from this set still
> > > used elsewhere?
> > 
> > The icon itself isn't removed in the tree but not packaged. When I tried to
> > remove a icon some time ago, Philipp didn't want I remove it from the tree.
> > Since then I leave them.
> 
> Philipp, is there still any reason why we should keep these not longer used
> png icons in the tree?

Hmm, can you remind me what reason I had? I can't think of a good reason off hand.

Comment 56

[Richard Marti (:Paenglab)]

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #55)
> (In reply to [:MakeMyDay] from comment #51)
> > Philipp, is there still any reason why we should keep these not longer used
> > png icons in the tree?
> 
> Hmm, can you remind me what reason I had? I can't think of a good reason off
> hand.

I don't know anymore why. It was during the XP theme removal. We still have all the old XP- and aero icons (toolbar-large.png and toolbar-small.png for example) and also the old icons in Linux and Mac. If you grant to remove all of them I could do this in a cleanup bug.

Comment 57

[Philipp Kewisch [:Fallen][📱📆]]

Sure, lets do that. Maybe you can zip the images up and attach them to the cleanup bug so we have another place they are kept than in hg history. I can imagine I didn't want to see them disappear completely.

Comment 58

[[:MakeMyDay]]

Comment on attachment 8996266 [details] [diff] [review]
Part 3: calendar-today.patch

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

r+ with removing calendar-overlay.png in the cleanup patch.

Comment 59

[[:MakeMyDay]]

Comment on attachment 8996267 [details] [diff] [review]
Part 4: calendar-alarm-icons.patch

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

r+ with comments considered and removing the not longer needed files in the cleanup patch.

::: calendar/base/content/calendar-month-view.xml
@@ +101,3 @@
>                          } else if (comp == 0) {
> +                            icon.setAttribute("type", "start");
> +                            label.value = "";

Why do you need to set an empty string here and above? Not setting it should have an equal result.

::: calendar/base/themes/common/calendar-views.css
@@ -91,4 @@
>  }
>  
>  .calendar-item-image {
> -    list-style-image: url(chrome://calendar-common/skin/day-box-item-image.png);

You didn't removed this file from the package, but this seems to be the only consumer.

@@ +101,5 @@
>      display: none;
>  }
>  
> +.multiday-view-main-box .calendar-item-image {
> +  margin-top: 4px;

indentation

Comment 60

[[:MakeMyDay]]

Comment on attachment 8996449 [details] [diff] [review]
Part 5: calendar-management.patch

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

r+ with removing calendar-status.png in the cleanup patch.

Comment 61

[[:MakeMyDay]]

Comment on attachment 8996466 [details] [diff] [review]
Part 6: calendar-statusbar-priority.patch

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

Thanks, looks good. r+ with removing calendar-event-dialog.png in the cleanup patch.

Comment 62

[[:MakeMyDay]]

Please still ask a mail peer for review of part1 regarding adding the icons to mail.

Can you then already prepare a version of part2 for ESR?

Comment 63

[Richard Marti (:Paenglab)]

Comment on attachment 8995798 [details] [diff] [review]
Part 1: calendar-tab-icons.patch

Magnus, is it okay to move the calendar ICO files to TB's ICOs? When they are in the extension they aren't shown in the titlebar nor in the task bar. And the default Windows icon would look like something is missing.

And Lightning is packaged by default, so this doesn't increase TB's package size.

Comment 64

[Richard Marti (:Paenglab)]

I filed bug 1480383 for the removal of all unneeded files.

Comment 65

[Magnus Melin [:mkmelin]]

(In reply to Richard Marti (:Paenglab) from comment #41)
> Comment on attachment 8995798 [details] [diff] [review]
> Part 1: calendar-tab-icons.patch
> 
> I forgot to mention. The ICO files in the extensions chrome directory don't
> apply to show as window icons in the task bar. I moved them now to the TB's
> ICO to be at the same place. 

Do we know why?

Comment 66

[Richard Marti (:Paenglab)]

No, I checked TB 52 and the icons aren't shown too. Maybe Windows checks only in this directory for the to the window names corresponding ICO files.

Comment 67

[Richard Marti (:Paenglab)]

Attachment: Part 4: calendar-alarm-icons.patch

Fixed review comments.

Comment 68

[Richard Marti (:Paenglab)]

Attachment: Part 5: calendar-management.patch

Part 5 needed a update after the Part 4 jar.mn changes.

Comment 69

[Richard Marti (:Paenglab)]

Attachment: Part 1: calendar-tab-icons-ESR.patch rebased for ESR

I'm asking only for this patch but this needs to apply for all patches.

Comment 70

[Richard Marti (:Paenglab)]

Attachment: IMIP-icon-ESR.patch

Rebased to apply on install.rdf instead of manifest.json.

Comment 71

[Magnus Melin [:mkmelin]]

Comment on attachment 8995798 [details] [diff] [review]
Part 1: calendar-tab-icons.patch

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

Not super thrilled about mixing those in here, but being pragmatic, r=mkmelin

Comment 72

[Richard Marti (:Paenglab)]

Thanks.
Jörg, when you check this in, can you change the commit message to "Bug 1446748 - Part 1: Use SVG icons for the tab favicons. r=MakeMyDay, mkmelin"?

Comment 73

[Jorg K (CEST = GMT+2)]

I think you need to clarify a few things here.

- You have 6 parts, right?
- Part 1 was supposedly reviewed by MMD and Magnus. I can't see MMD's review anywhere.
  Besides, why are we packaging Calendar things into
  mail/app/Makefile.in and mail/installer/package-manifest.in. Magnus isn't "super thrilled"
  if I interpret comment #71 correctly and neither am I. Where it the explanation/justification for this?
  Oh, comment #63 and below. Which title/taskbar does that refer to? Something in Windows? And what is missing
  in TB 52?
- ESR: ESR was never applied for, yet there is an ESR patch for part 1 with approval
  and a IMIP-icon-ESR.patch (part 2) without approval.
  What about the other parts? What about beta? - Or have we given up beta 62?

Comment 74

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #73)
> I think you need to clarify a few things here.
> 
> - You have 6 parts, right?
> - Part 1 was supposedly reviewed by MMD and Magnus. I can't see MMD's review
> anywhere.

It was reviewed in comment 48.

>   Besides, why are we packaging Calendar things into
>   mail/app/Makefile.in and mail/installer/package-manifest.in. Magnus isn't
> "super thrilled"
>   if I interpret comment #71 correctly and neither am I. Where it the
> explanation/justification for this?
>   Oh, comment #63 and below. Which title/taskbar does that refer to?
> Something in Windows? And what is missing
>   in TB 52?

If you know how to make Windows look in the extensions default directory for the ICOs, then I'll use this immediately.

> - ESR: ESR was never applied for, yet there is an ESR patch for part 1 with
> approval
>   and a IMIP-icon-ESR.patch (part 2) without approval.
>   What about the other parts? What about beta? - Or have we given up beta 62?

See comment 69: I only asked with Part 1 for all patches to not create too much mails.

Comment 75

[Jorg K (CEST = GMT+2)]

Ah, instead of asking for additional review you turned MMD's r+ into an r?mkmelin. OK.

Once again, where are the icons missing? Already in TB 52? Which title/taskbar?

Comment 76

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #75)
> Ah, instead of asking for additional review you turned MMD's r+ into an
> r?mkmelin. OK.
> 
> Once again, where are the icons missing? Already in TB 52? Which
> title/taskbar?

For example the when you have a event dialog, where is no icon on top left. Also in the Windows task bar, the little preview windows have not the correct icon, only the default Windows icon.

Comment 77

[Jorg K (CEST = GMT+2)]

I use "event in a tab" and I switched off the Windows taskbar preview ;-) - Yes, if you want full Windows integration, what you have is possibly the only way. Besides, Lightning is shipped as packed XPI these days, so there is no extensions folder.

Comment 78

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f53db719823b
Part 1: Use SVG icons for the tab favicons. r=MakeMyDay,mkmelin
https://hg.mozilla.org/comm-central/rev/1efbc803999a
Part 2: Use a SVG icons for the IMIP bar. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/6911f3567880
Part 3: Use SVG icons for the today pane. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/a8feed690990
Part 4: Use SVG icons for the reminder icons. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/0a2895b12bf0
Part 5: Use SVG icons for the Calendar management. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/99d901a686dd
Part 6: Use SVG icons for the event status bar priority. r=MakeMyDay

Comment 79

[Jorg K (CEST = GMT+2)]

Comment on attachment 8996996 [details] [diff] [review]
IMIP-icon-ESR.patch

(In reply to Richard Marti (:Paenglab) from comment #70)
> Rebased to apply on install.rdf instead of manifest.json.
I wish. This is identical to the trunk patch.

Comment 80

[Richard Marti (:Paenglab)]

Comment on attachment 8996996 [details] [diff] [review]
IMIP-icon-ESR.patch

(In reply to Jorg K (GMT+2) from comment #79)
> Comment on attachment 8996996 [details] [diff] [review]
> IMIP-icon-ESR.patch
> 
> (In reply to Richard Marti (:Paenglab) from comment #70)
> > Rebased to apply on install.rdf instead of manifest.json.
> I wish. This is identical to the trunk patch.

It can't because ESR doesn't use manifest.json for the extensions. Check the ESR tree and you find no manifest.json but the install.rdf.

Comment 81

[Richard Marti (:Paenglab)]

Attachment: Part 2: IMIP-icon-ESR.patch

Correct ESR patch

Comment 82

[Jorg K (CEST = GMT+2)]

TB 60 ESR, Cal 6.2:
https://hg.mozilla.org/releases/comm-esr60/rev/13fbff75e2e5350946dd2d412ad336651954943f
https://hg.mozilla.org/releases/comm-esr60/rev/531d000db9186a6a923393f113afb802c856e427 (*)
https://hg.mozilla.org/releases/comm-esr60/rev/a222a8a41496a1f813708d5bbf63045cf95a9706
https://hg.mozilla.org/releases/comm-esr60/rev/aff8b23dd84d3cbe31023bf4f43ded7f6573b61d
https://hg.mozilla.org/releases/comm-esr60/rev/3895b52fa707559f10d76960cd1c01922a218c20
https://hg.mozilla.org/releases/comm-esr60/rev/44ff9ed4b99d7a9a25cb31d1d2b83841f7b30d9a
(*) fixed the install.rdf file: <em:iconURL>chrome://calendar-common/skin/icons/icon32.svg</em:iconURL>

Comment 83

[Jorg K (CEST = GMT+2)]

Attachment: Something wrong with the edit icon

I'm running a freshly built TB 60 ESR and noticed something wrong with the edit even icon.

From an accepted invitation, click details in the notification bar and a window opens up with some event details. The icon has two dots whereas the icon when creating or editing an event (obviously not in a tab) doesn't have those dots. The screenshot shows both.

While we're here: The icons on the Windows taskbar are too big, they are too much in your face. The reminder icon, the bell, isn't really sharp.

Comment 84

[[:MakeMyDay]]

Thanks for chaecking. Regarding the icons looking sharp, see comment 49 I had a similar issue using a scaled resolution.

Richard can you have a look at the dots and maybe to the size of the window bar icon, however  I think it is still acceptable. It just looks that big since it more a box then a circle like the TB icon.

Comment 85

[Jorg K (CEST = GMT+2)]

I'm using a 1280x1024 unscaled display. Only the bell isn't sharp.

Comment 86

[Jorg K (CEST = GMT+2)]

OK, let's forget the fuzziness, it's because the bell is round and the rastering isn't optimal. The items look big and bold because they fill the entire space, unlike the circular TB icon, as MMD already said in comment #84.

However, the two extra pixels I pointed out aren't right.

Comment 87

[Jorg K (CEST = GMT+2)]

Attachment: Something wrong with the fill on the right side

If you have one event with a name longer than the others, the space to the right of the bell for the shorter-named events is filled incorrectly.

Comment 88

[[:MakeMyDay]]

(In reply to Jorg K (GMT+2) from comment #87)
> Created attachment 8997636 [details]
> Something wrong with the fill on the right side
> 
> If you have one event with a name longer than the others, the space to the
> right of the bell for the shorter-named events is filled incorrectly.

Thanks for finding this. Since that was not the main scope of this bug and fixing could be complex, I would be fine if we take care of that in a follwo-up bug. Do you mind to file one?

Comment 89

[Richard Marti (:Paenglab)]

The two dots in the icon is a error in the ICO. I'm on fixing it. I make them also smaller with a 1px empty border around them.

Comment 90

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #87)
> Created attachment 8997636 [details]
> Something wrong with the fill on the right side
> 
> If you have one event with a name longer than the others, the space to the
> right of the bell for the shorter-named events is filled incorrectly.

This is no bug, this is PEBCAK. You have set the birthday category to use the almost same color than the calendar itself. Change the color of one of the two and you'll see it.

Comment 91

[Richard Marti (:Paenglab)]

Attachment: calendar-smaller-icons.patch

There was a graphical glitch in the calendar-event-summary-dialog.png.

I made the icons now a bit smaller by not surrounding them with the white background. I also added a 32px icon to look better when the big icons are chosen for the Windows task bar.

Comment 92

[[:MakeMyDay]]

Comment on attachment 8997643 [details] [diff] [review]
calendar-smaller-icons.patch

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

Thanks,looks good.

Comment 93

[[:MakeMyDay]]

I forgot to mention: I'd like to be the smaller icon patch the last change in this bug. If anything else comes up, please file follow-up bugs.

Comment 94

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/763cca5220a9
Make the Windows icons a bit smaller. r=MakeMayDay DONTBUILD

Comment 95

[Jorg K (CEST = GMT+2)]

Much better now, thanks.

Comment 96

[Jorg K (CEST = GMT+2)]

Follow-up:
https://hg.mozilla.org/releases/comm-esr60/rev/cb9cd7151c41923ed301db46f864b0d7cdf8a3e9