Open Bug 1683865 Opened 7 months ago Updated 7 days ago

[meta] replace <xul:image> usage in Thunderbird

Categories

(Thunderbird :: General, task)

Tracking

(thunderbird_esr78 wontfix)

ASSIGNED
88 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: mkmelin, Assigned: henry)

References

(Depends on 2 open bugs)

Details

(Keywords: leave-open, meta)

Attachments

(48 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
13.20 KB, image/png
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

This bug tracks removal of xul:image usage.

mail: https://searchfox.org/comm-central/search?q=%3Cimage&path=mail&case=false&regexp=false ~140 cases

calendar: https://searchfox.org/comm-central/search?q=%3Cimage&path=calendar&case=false&regexp=false 29 hits

See bug 1584641 for removal of -moz-image-region, and bug 1403231 comment 2, which is quite intertwined with this bug, and should also be covered here.

It seems best not to invent a new element for images since there is already img.

There's a mix of easy and harder cases to take care of here. I think we could do the fairly easy ones first, than think about the hard ones later.

The easy ones is where the image has the same url all the time for all themes across the platforms. Seems this can generally be converted to <html:img src=... />. Cases like .person-icon, #userIcon, #attachmentIcon, the stuff in emailWizard.xhtml.

Note that <html:img> can't use content: url();

I didn't check the -moz-image-region parts, but they should each be attackable.

Assignee: nobody → henry
Mentor: alessandro
Status: NEW → ASSIGNED
Target Milestone: --- → 88 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/11efbeb3d116
convert <xul:image> usage to <html:img> in image_editContactPanel.inc.xhtml. r=aleca

Has any consideration been given as to whether css background-image on an html element (could be img, or div etc.) is more advantageous than using the src attribute and thus needing <html:img> specifically?

Yes, see bug 1584641, and we will likely have to do that at least for certain situations (different image per theme at least). But, in general <img> is the semantically correct solution so other than a slightly higher cost for the one-time move over, it seems preferable.

You could also use in CSS the background-image and padding with the half values of the width and height. Like

#link-image-top {
 background-image: url(chrome://calendar/skin/shared/link-image-top.svg);
 padding: 3.5px 4.5px;
}

(In reply to alta88 from comment #5)

Has any consideration been given as to whether css background-image on an html element (could be img, or div etc.) is more advantageous than using the src attribute and thus needing <html:img> specifically?

The main difference of the background-image approach is that the source can remain specified in css. This can also be done with css content, but I think for background-image you would have to specify some dimensions in order for the element to not collapse. I would prefer content on a div, since background-image should probably be a background to something else.

Saying that, when making some edits for this bug, I have seen background-image being used for icons. I think it was to create a cropping effect that changes with a change in attributes, but this can be done with css object-fit and object-position (maybe it wasn't an option at the time?).

However, I still prefer using html:img with a set src because it is the obvious html element to use for images. This requires more work when the source is meant to change with different attribute values (which is simpler with css selectors) but I think its worth it.

It's the same with background-image and content. You have to set the dimensions and this is the easiest with the padding trick.

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

It's the same with background-image and content. You have to set the dimensions and this is the easiest with the padding trick.

ok, I wasn't sure about content.

I don't understand why padding would be better than width and height, especially since it obscures what you actually want. Is it so it also works on display: inline elements?

I had some issues when I used width/height. When it works for you then it's okay to use it.

There are 2 issues:

  1. <img> may seem like it should be used, because it's called img; that doesn't mean it's technically better just 'feels better', and this seems to be the reason it's being supported here. There should be a stronger reason. Some stronger reasons don't apply to the use case here: internal chrome pages, not web pages. The sentiment in Bug 1584641 doesn't seem pro img but pro css content.
  2. The larger objection is to using the src attribute. This is definitely more work, maintenance wise. Loading a file as src allows the parser to call an element's onerror for bad images. This is not free, and certainly not a necessary feature for internal pages. Most importantly though, a src image cannot be overridden in css.

There's not necessarily one single right answer that covers all cases. For cases where it's not purely for design, e.g. it's an image you can/should click, <img> is needed for accessibility purposes. Cases where things wouldn't work correctly if the image is missing or couldn't load.
It's quite conceivable we'd load the pages also through other means than internal chrome some time in the future, and there could then be some possibility an image didn't load due to networking errors or similar.

<img src> can be overridden in css if desired - see http://www.audenaerde.org/csstricks.html#imagereplacecss

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

There's not necessarily one single right answer that covers all cases. For cases where it's not purely for design,

But that will be the case for the very large majority of conversions. It is the case for both the patch checked in and the patch queued up. Using src for the editContactPanel panel star is the least correct and efficient way to do it. Tb should follow Emilio's suggestion in Bug 1584641 for content else use background-image.

Cases where things wouldn't work correctly if the image is missing or couldn't load.
It's quite conceivable we'd load the pages also through other means than internal chrome some time in the future, and there could then be some possibility an image didn't load due to networking errors or similar.

There is no legitimate scenario for missing image to be true for internal pages and I don't see how remote loading is conceivable at all; no one would propose such a thing for a desktop app. Some hypothetical thunderbird on the web scenario is not relevant here, and even then would be delivered via a font library with unicode private area definitions using content, like the tag button on this page.

<img src> can be overridden in css if desired - see http://www.audenaerde.org/csstricks.html#imagereplacecss

Sure, that is what's called a paper-over masking hack, not an override.

(In reply to alta88 from comment #12)

  1. The larger objection is to using the src attribute. This is definitely more work, maintenance wise. Loading a file as src allows the parser to call an element's onerror for bad images. This is not free, and certainly not a necessary feature for internal pages. Most importantly though, a src image cannot be overridden in css.

Is the idea that you think the image for an icon should be treated as styling, and therefore remain in css as much as possible? Similar to how button and selector icons are specified in css with list-style-image?

Also, do you have an example in mind for where you would want to override in css?

Side note

It would be nice if there was a general policy on what element to use in different circumstances. For example, using some examples I've come across in calendar:

  1. Elements with no image, but used for their css decorations. E.g. the calendar colors. If it doesn't have some accessibility role, probably want an empty div.
  2. Clickable images. E.g. the lock chain icon when setting event times. Probably want an img with alt describing the action. src would need to be set since css content doesn't work on img.
  3. Images that convey information. E.g. the calendar alarm and private icons that act as flags. Probably want an img with an appropriate alt.
  4. Images that complement an action. E.g. the timezone map (both the map itself, and the highlight overlay). Probably want an img with alt="" so that it can be ignored by accessibility.
  5. Images that only decorate. E.g. the L linkers that sit above and below the lock chain icon when setting event times. This could either be an empty div with set css content or an img with alt="".
  6. Images that 'spoof' a button icon, next to some text. E.g. the arrow next to 'Today Pane' in the status bar. Same as above, could either be an empty div or an img with alt="".
  7. Images that have their source set in javascript. E.g. the file-type icons for attachments. This is easiest with setting src on an img, but you could technically have a div and set divElement.style.content = "url()" in javascript. But even then, you wouldn't be able to override in css (unless you used important!?).

At the moment, accessibility in calendar is probably very poor in general, so I'm not sure how much difference alt text would make. But it would be good to work in that direction. (Good news: this discussion has made me think about this more, so I will be adding alt to my changes). Please let me know if I've got something wrong regarding using alt for accessibility in thunderbird.

@alta88 Maybe it would help to talk about specific cases, as in the above. Are you advocating for using an empty div with css content for some of these cases?

Now only use a single html:img for showing the priority status.
The svg was split into three separate images.

The 'image' element in <stack><image id="mini-day-image"/><hbox/></stack> was only used for its CSS background, but this can be applied to the 'hbox' directly, so the 'stack' and 'image' were removed and the 'id' was placed in the 'hbox'.

Also changed the drag 'xul:image' element to a 'html:img'.

Attachment #9212119 - Attachment description: WIP: Bug 1683865 - Replace xul:image usage in event item classification and category. r=darktrojan → Bug 1683865 - Replace xul:image usage in event item classification and category. r=darktrojan

Also removed unused associated attributes: 'allday', 'progress', 'itemType' and 'todoType'.

Depends on D110060

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/bbfbd335b145
Replace xul:image usage in event edit dialog. r=darktrojan
https://hg.mozilla.org/comm-central/rev/73ae14c4f53d
Replace xul:image usage in calendar alarm dialog. r=darktrojan
https://hg.mozilla.org/comm-central/rev/366832b3cd41
Replace xul:image usage in email event invitation bar. r=darktrojan
https://hg.mozilla.org/comm-central/rev/68e5144c9b49
Replace xul:image usage for event priority bars. r=darktrojan
https://hg.mozilla.org/comm-central/rev/96a872a4ac6f
Replace xul:image usage in today pane. r=darktrojan
https://hg.mozilla.org/comm-central/rev/c56017612648
Replace xul:image usage in event item classification and category. r=darktrojan

The 'status-icon' class was completely removed since it was unused.

Also replaced the 'set parentorient' with a watch on the 'parentorient' and 'whichside' attributes. The former method was not working: the 'parentorient' property was always being set to "" in 'connectedCallback' because the 'parentorient' attribute had not yet been set.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/d4afbf8aaef6
Replace xul:image usage in event item. r=darktrojan
https://hg.mozilla.org/comm-central/rev/e9863d2e6541
Replace xul:image usage in calendar agenda summary. r=darktrojan
https://hg.mozilla.org/comm-central/rev/052f22f50429
Replace xul:image usage in calendar summary. r=darktrojan
https://hg.mozilla.org/comm-central/rev/e5f34a08a2d5
Replace xul:image usage in attendance dialog. r=darktrojan
https://hg.mozilla.org/comm-central/rev/8cbb622a3cd7
Replace xul:image usage in network calendar creation. r=darktrojan
https://hg.mozilla.org/comm-central/rev/2e851450f0db
Replace xul:image usage in event timezone dialog. r=darktrojan
https://hg.mozilla.org/comm-central/rev/fdeeaa5b4cd2
Replace xul:image usage in event attachments. r=darktrojan
https://hg.mozilla.org/comm-central/rev/3a15062552f2
Replace xul:image usage in setting event reminders. r=darktrojan
https://hg.mozilla.org/comm-central/rev/c3d7a5ec0fd5
Replace xul:image usage in calendar manager. r=darktrojan
https://hg.mozilla.org/comm-central/rev/1037586298d6
Replace xul:image usage in attendee dialog. r=darktrojan
https://hg.mozilla.org/comm-central/rev/b69844efd68a
Replace xul:image usage in event gripbars. r=darktrojan
https://hg.mozilla.org/comm-central/rev/a21c8adc678f
Replace xul:image usage in statusbar today pane. r=darktrojan

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/4779e6740711
Backed out changeset 3a15062552f2 for test failures in at least calendar/test/unit/test_alarmutils.js

One of the patches seemed to have regressed the indicator-private-browsing.svg icon used in the calendar sidebar list.
The "lock" icon appears when a calendar is marked as read-only.

Mentor: alessandro

Just to clarify, the icon is still there but the image is empty when the calendar is not read-only.
So the CSS should be updated to hide the image, I think.

Otherwise the image will be visible with a black outline.

(In reply to Alessandro Castellani [:aleca] from comment #37)

Just to clarify, the icon is still there but the image is empty when the calendar is not read-only.
So the CSS should be updated to hide the image, I think.

Yup, it should be invisible. I added a patch.

Weirdly I tested this specific case visually before and missed it. I know for some of the other changesets where the src attribute is removed I set them to be display: none when I didn't want them to take up space, but I'll double check the other cases.

Just checked the other patches, all other cases of no src attribute would be error cases (so the black square would be an appropriate indicator) or are hidden by the parent.

Attachment #9215056 - Attachment description: Bug 1683865 - Replace xul:image usage in setting event reminders. r=darktrojan → Bug 1683865 - Replace xul:image usage in setting event reminders. r=darktrojan!

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/67263bb1748d
Make calendar-readstatus invisible when it has no src. r=aleca

Attachment #9215056 - Attachment description: Bug 1683865 - Replace xul:image usage in setting event reminders. r=darktrojan! → Bug 1683865 - Replace xul:image usage in setting event reminders. r=darktrojan

Also, as part of this:

  • Reduced styling repetitions, and some coding repetitions.
  • Dropped some stylesheets because they were no longer needed or the few rules could be merged.
  • Moved logic related to chat-conversation-info into internal methods.
  • Removed some old styling hacks that were used to show the user name and status in chat-tooltip, chat-conversation-info and imAccounts, and replaced them with a grid.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/61591773c2d0
Replace xul:image usage in setting event reminders. r=darktrojan

Attachment #9219002 - Attachment description: Bug 1683865 - Replace xul:image usage for chat user, protocol and status icons. r=clokep → Bug 1683865 - Replace xul:image usage for chat user, protocol and status icons. r=freaktechnik

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/9c30d915b033
Replace xul:image usage for chat user, protocol and status icons. r=freaktechnik

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/32100bb1fe5f
Replace xul:image usage in OTR fingerprint dialog. r=freaktechnik
https://hg.mozilla.org/comm-central/rev/d2b3fe4417d6
Replace xul:image usage in chat roles. r=freaktechnik
https://hg.mozilla.org/comm-central/rev/6b9339177659
Replace xul:image usage in chat groups. r=freaktechnik

Regressions: 1709678

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/885b6b740086
Fix setting an error icon for calendar cloud attachment failure. r=darktrojan

There are a few places that use the loading.png icon to indicate waiting (normally for the network). Each of these can be changed to

<img src="chrome://global/skin/icons/loading.png"
     srcset="chrome://global/skin/icons/loading@2x.png 2x"
     alt="" />

However, since this would be fairly common, I thought it might make sense to create a custom HTML element <loading-img/>, which just sets these attributes by default.

But I have two questions:

  1. Is creating a custom element to only set some common attributes over the top? Is there a lighter method to get the same result?
  2. Which directory is a good place for such custom elements that are used across TB?
Flags: needinfo?(mkmelin+mozilla)

Sorry to interject here, but we should slowly getting rid of the loading PNG and switch to the SVG version.
The PNG forces us to have a 2x variation for HiDPI monitors, and we can't control it to properly adapt it for dark theme variations.

We currently use the new SVG with a CSS based animation in a couple of areas.
https://searchfox.org/comm-central/rev/571b1a21fbcb0e57b10a931a4c599a6fb0cb7292/mail/themes/shared/mail/tabmail.css#214-238
https://searchfox.org/comm-central/rev/571b1a21fbcb0e57b10a931a4c599a6fb0cb7292/mail/themes/shared/mail/message-bar.css#147-171

Not sure if this is out of scope for this bug, but I just wanted to put it out there to avoid for Henry to do double work.

(In reply to Henry Wilkes [:henry] from comment #51)

  1. Is creating a custom element to only set some common attributes over the top? Is there a lighter method to get the same result?

CE is probably how to do it. But I would say it's not worth doing if the functionality is minimal, like for the images. CEs are very expensive compared to normal elements, and if things ever change, global search/replace is very easy/fast to do, so not something to worry too much about.

  1. Which directory is a good place for such custom elements that are used across TB?

They would be with the other mail/ widgets. Geoff is currently moving those to a separate folder.

Like :aleca says, we want to use svgs going forwards, but e.g. your 2x case is easily fixable by a global search/replace at that point.

Flags: needinfo?(mkmelin+mozilla)

Thanks for the info. And its useful to know that we'll be switching to svgs.

I'll stick with the basic method rather than use custom elements.

Also allowed the chat status icon to show a wider range of statuses.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/b296c12d6577
Replace xul:image usage in email address headers. r=mkmelin

In cases where we (un)set the alt text through a mixture of l10n.setAttributes, setAttribute("alt",) and removeAttribute("alt"), we make sure to remove the data-l10n-id attribute in the last two cases.

This cleans up the element's attributes (which is useful for debugging inspection) but is also needed in some cases. For example, if we perform the following sequence:

  1. l10n.setAttributes(image, "same-fluent-id-to-set-alt")
  2. image.setAttribute("alt", "")
  3. l10n.setAttributes(image, "same-fluent-id-to-set-alt")

The alt attribute is not restored by the last call because the data-l10n-id attribute is unchanged. We need to remove the data-l10n-id attribute after step 2 so that the localization can detect the change in id and restore the alt text.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/e449dd990478
Remove the data-l10n-id when setting alt text through both fluent and inline. r=mkmelin

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/5803f56195b0
Replace xul:image usage in treecol-image. r=mkmelin

Removed the unused icon part.

Dropped the 32 image size since the icon is only ever displayed as 16px by 16px.

The button was constructed as a xul label, but its event behaviour and role were set to mimic that of a button. So a button is more semantically correct and brings the desired behaviour automatically.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/ab0cf15447b2
Replace xul:image usage in menulist-editable. r=mkmelin

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/3dc8a1620918
Replace xul:image usage in attachment list. r=mkmelin
https://hg.mozilla.org/comm-central/rev/0a4c15818b12
Replace xul:image usage in address pills. r=mkmelin
https://hg.mozilla.org/comm-central/rev/760cf06249e1
Convert the addresses field close button from a xul:label into a xul:button. r=mkmelin
https://hg.mozilla.org/comm-central/rev/87d37b027784
Replace xul:image usage in toolbar activity indicator. r=mkmelin

All the textual information in the icon tooltip is already available in the status box adjacent to it.

Current specifications (see "Accessible Name and Description Computation" in HTML Accessibility API Mappings 1.0) state that if there is an alt attribute for an img it is used as the Accessible Name, otherwise the title attribute is used as the Accessible Name. See also Bug 1700174.

For existing situations where the title describes an icon for visual pointer users, and we also want this to be the Accessible Name, to avoid duplicating information, we should simply not include the alt attribute.

Note that these situations should still be avoided because a visual keyboard user would not have access to the title text.

Regressions: 1713426

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1c0c6a085c8f
Remove alt text that duplicates the title. r=mkmelin

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/668a02c0a6db
Remove title from chat-conversation-info status icon. r=freaktechnik

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/16972bd72bf2
Replace xul:image usage in message encryption header. r=mkmelin
https://hg.mozilla.org/comm-central/rev/4a6f152a44d9
Replace xul:image usage in tab drop indicator. r=mkmelin

(In reply to Henry Wilkes [:henry] from comment #40)

Just checked the other patches, all other cases of no src attribute would be error cases (so the black square would be an appropriate indicator) or are hidden by the parent.

<img class="reminder-icon" value="NONE" alt="" />

I think we should just hide this case, it's not an error and not having an icon is appropriate. (Code in calendar/base/content/dialogs/calendar-event-dialog-reminder.js.)

(In reply to Geoff Lankow (:darktrojan) from comment #75)

(In reply to Henry Wilkes [:henry] from comment #40)

Just checked the other patches, all other cases of no src attribute would be error cases (so the black square would be an appropriate indicator) or are hidden by the parent.

<img class="reminder-icon" value="NONE" alt="" />

I think we should just hide this case, it's not an error and not having an icon is appropriate. (Code in calendar/base/content/dialogs/calendar-event-dialog-reminder.js.)

I didn't realise other values were allowed. What does value="NONE" mean in the reminder dialog?

Looking through now, it seems "AUDIO" is checked in CalAlarm.jsm. I also remember removing a test that used "MOBILE".

Where do these other values come from since the selector in the reminder dialog only has "EMAIL" and "DISPLAY" (https://searchfox.org/comm-central/rev/b972d9ae1bd20536044be70c5bf3670e70b40720/calendar/base/content/dialogs/calendar-event-dialog-reminder.xhtml#124). Is there a limited set of values somewhere?

Would it make sense to have icons for these other cases? Currently in the reminder dialog, for "DISPLAY", it shows

<alert-icon> | 15 minutes before the event starts

or in the accessibility tree

Show an Alert | 15 minutes minutes before the event starts

But without an icon, it would just be

15 minutes before the event starts

without any idication of what the alert type is, or what the text refers to. So maybe we should have icons for each case, or at least use a fallback icon (like a question mark with alt text Unknown Alert).

In the case of event boxes where we only have an icon on its own (https://searchfox.org/comm-central/rev/b972d9ae1bd20536044be70c5bf3670e70b40720/calendar/base/modules/utils/calAlarmUtils.jsm#140), would you rather use a fallback icon? Otherwise, we might as well not create and add the <img> at all.

https://icalendar.org/iCalendar-RFC-5545/3-8-6-1-action.html - it's not a fully limited set. But the ones we don't support MUST be ignored.

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

https://icalendar.org/iCalendar-RFC-5545/3-8-6-1-action.html - it's not a fully limited set. But the ones we don't support MUST be ignored.

Does that mean in setupListItem (https://searchfox.org/comm-central/rev/b972d9ae1bd20536044be70c5bf3670e70b40720/calendar/base/content/dialogs/calendar-event-dialog-reminder.js#210) we should not create the list item if it is neither "DISPLAY" nor "EMAIL"?

Well, and AUDIO. But otherwise correct.

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

Well, and AUDIO. But otherwise correct.

OK, I'll make the changes. We also would need an "AUDIO" icon then, or at least include some additional text, otherwise we still have this problem from comment #76:

without an icon, it would just be

15 minutes before the event starts

without any idication of what the alert type is, or what the text refers to.

(In reply to Henry Wilkes [:henry] from comment #80)

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

Well, and AUDIO. But otherwise correct.

OK, I'll make the changes. We also would need an "AUDIO" icon then, or at least include some additional text, otherwise we still have this problem from comment #76:

There is one at "chrome://global/skin/media/audio.svg".

DISPLAY and EMAIL actions already had icons, and added icon support for the AUDIO action. All other actions should be ignored by the UI until they are supported.

Attachment #9227149 - Attachment description: Bug 1683865 - Replace xul:image usage in message attachment icon. r=mkmelin → Bug 1683865 - Replace xul:image usage in message attachment. r=mkmelin

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/4a72aca99202
Do not show reminder icons for unsupported reminder actions. r=darktrojan
https://hg.mozilla.org/comm-central/rev/46839b999e58
Replace xul:image usage in content pane security icon. r=mkmelin
https://hg.mozilla.org/comm-central/rev/4785f8e16605
Replace xul:image usage in message attachment. r=mkmelin
https://hg.mozilla.org/comm-central/rev/7b9edcbe3bf7
Replace xul:image usage in openpgp notification icons. r=mkmelin
https://hg.mozilla.org/comm-central/rev/2d19637d6b1f
Replace xul:image usage in openpgp settings. r=mkmelin

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/cd9a0fda9cf8
followup to fix Fluent linting. rs=fluent-lint DONTBUILD
Depends on: 1716759

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/aa4a4b2932ac
Replace xul:image usage in openpgp key generation/import. r=mkmelin

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/d392076e308e
Replace xul:image usage in sanitation dialog. r=mkmelin

The activity manager richlistitems were simplified to use html elements instead of xul. Common CSS between platforms was merged into a shared file.

The unused activity buttons (undo, pause, resume, restart, cancel and recover) were dropped.

Also changed the representation of the extra buttons from a xul:panel to an in-place set of buttons that are shown/hidden to reflect the semantics: neither a dialog nor a selection menu, but an overflow for the set of buttons.

Also fixed the arrow key navigation in this area, so that it is possible to smoothly navigate between the main set of buttons, and the overflow set.

(In reply to Henry Wilkes [:henry] from comment #95)

Created attachment 9231185 [details]
Bug 1683865 - Replace "add recipient field" xul elements with html buttons. r=aleca

I'm not sure this patch should be in this bug as it does way more than only replacing the xul:image.
I think it's better to track this in a dedicated bug.

Depends on: 1721352
You need to log in before you can comment on or make changes to this bug.