[meta] replace <xul:image> usage in Thunderbird
Categories
(Thunderbird :: General, task)
Tracking
(thunderbird_esr78 wontfix, thunderbird_esr102 unaffected)
Tracking | Status | |
---|---|---|
thunderbird_esr78 | --- | wontfix |
thunderbird_esr102 | --- | unaffected |
People
(Reporter: mkmelin, Assigned: elizabeth)
References
(Blocks 1 open bug)
Details
(Keywords: meta)
Attachments
(73 files, 2 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
Bug 1683865 - Convert the addresses field close button from a xul:label into a xul:button. r=mkmelin
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
This bug tracks removal of xul:image usage.
mail: https://searchfox.org/comm-central/search?q=%3Cimage&path=mail&case=false®exp=false ~140 cases
calendar: https://searchfox.org/comm-central/search?q=%3Cimage&path=calendar&case=false®exp=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.
Reporter | ||
Comment 1•4 years ago
|
||
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.
Reporter | ||
Comment 2•4 years ago
|
||
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
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
Comment 4•4 years ago
|
||
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?
Reporter | ||
Comment 6•4 years ago
|
||
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.
Comment 7•4 years ago
|
||
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;
}
Comment 8•4 years ago
|
||
(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 thesrc
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.
Comment 9•4 years ago
|
||
It's the same with background-image
and content
. You have to set the dimensions and this is the easiest with the padding
trick.
Comment 10•4 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #9)
It's the same with
background-image
andcontent
. You have to set the dimensions and this is the easiest with thepadding
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?
Comment 11•4 years ago
|
||
I had some issues when I used width/height. When it works for you then it's okay to use it.
Comment 12•4 years ago
|
||
There are 2 issues:
<img>
may seem like it should be used, because it's calledimg
; 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 proimg
but pro csscontent
.- The larger objection is to using the
src
attribute. This is definitely more work, maintenance wise. Loading a file assrc
allows the parser to call an element'sonerror
for bad images. This is not free, and certainly not a necessary feature for internal pages. Most importantly though, asrc
image cannot be overridden in css.
Reporter | ||
Comment 13•4 years ago
|
||
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
Comment 14•4 years ago
|
||
(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.
Comment 15•4 years ago
|
||
(In reply to alta88 from comment #12)
- The larger objection is to using the
src
attribute. This is definitely more work, maintenance wise. Loading a file assrc
allows the parser to call an element'sonerror
for bad images. This is not free, and certainly not a necessary feature for internal pages. Most importantly though, asrc
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:
- 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
. - Clickable images. E.g. the lock chain icon when setting event times. Probably want an
img
withalt
describing the action.src
would need to be set since csscontent
doesn't work onimg
. - Images that convey information. E.g. the calendar alarm and private icons that act as flags. Probably want an
img
with an appropriatealt
. - Images that complement an action. E.g. the timezone map (both the map itself, and the highlight overlay). Probably want an
img
withalt=""
so that it can be ignored by accessibility. - 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 emptydiv
with set csscontent
or animg
withalt=""
. - 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 animg
withalt=""
. - Images that have their source set in javascript. E.g. the file-type icons for attachments. This is easiest with setting
src
on animg
, but you could technically have adiv
and setdivElement.style.content = "url()"
in javascript. But even then, you wouldn't be able to override in css (unless you usedimportant!
?).
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?
Comment 16•4 years ago
|
||
Comment 17•4 years ago
|
||
Comment 18•4 years ago
|
||
Now only use a single html:img for showing the priority status.
The svg was split into three separate images.
Comment 19•4 years ago
|
||
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'.
Comment 20•4 years ago
|
||
Updated•4 years ago
|
Comment 21•4 years ago
|
||
Also removed unused associated attributes: 'allday', 'progress', 'itemType' and 'todoType'.
Depends on D110060
Comment 22•4 years ago
|
||
Depends on D110267
Updated•4 years ago
|
Comment 23•4 years ago
|
||
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
Comment 24•4 years ago
|
||
Comment 25•4 years ago
|
||
Comment 26•4 years ago
|
||
Comment 27•4 years ago
|
||
Comment 28•4 years ago
|
||
Comment 29•4 years ago
|
||
Comment 30•4 years ago
|
||
Comment 31•4 years ago
|
||
The 'status-icon' class was completely removed since it was unused.
Comment 32•4 years ago
|
||
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.
Comment 33•4 years ago
|
||
Updated•4 years ago
|
Comment 34•4 years ago
|
||
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
Comment 35•4 years ago
|
||
Comment 36•4 years ago
|
||
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.
Updated•4 years ago
|
Comment 37•4 years ago
|
||
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.
Comment 38•4 years ago
|
||
Otherwise the image will be visible with a black outline.
Comment 39•4 years ago
|
||
(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.
Comment 40•4 years ago
|
||
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.
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 41•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/67263bb1748d
Make calendar-readstatus invisible when it has no src. r=aleca
Updated•4 years ago
|
Updated•4 years ago
|
Comment 42•4 years ago
|
||
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.
Comment 43•4 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/61591773c2d0
Replace xul:image usage in setting event reminders. r=darktrojan
Updated•4 years ago
|
Updated•4 years ago
|
Comment 44•4 years ago
|
||
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
Comment 45•4 years ago
|
||
Comment 46•4 years ago
|
||
Comment 47•4 years ago
|
||
Updated•4 years ago
|
Comment 48•4 years ago
|
||
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
Comment 49•4 years ago
|
||
Updated•4 years ago
|
Comment 50•4 years ago
|
||
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
Comment 51•4 years ago
|
||
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:
- Is creating a custom element to only set some common attributes over the top? Is there a lighter method to get the same result?
- Which directory is a good place for such custom elements that are used across TB?
Comment 52•4 years ago
|
||
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.
Reporter | ||
Comment 53•4 years ago
|
||
(In reply to Henry Wilkes [:henry] from comment #51)
- 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.
- 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.
Comment 54•4 years ago
|
||
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.
Comment 55•4 years ago
|
||
Also allowed the chat status icon to show a wider range of statuses.
Updated•4 years ago
|
Comment 56•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/b296c12d6577
Replace xul:image usage in email address headers. r=mkmelin
Comment 57•4 years ago
|
||
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:
- l10n.setAttributes(image, "same-fluent-id-to-set-alt")
- image.setAttribute("alt", "")
- 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.
Comment 58•4 years ago
|
||
Reporter | ||
Updated•4 years ago
|
Comment 59•4 years ago
|
||
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
Comment 60•4 years ago
|
||
Updated•4 years ago
|
Comment 61•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/5803f56195b0
Replace xul:image usage in treecol-image. r=mkmelin
Comment 62•4 years ago
|
||
Removed the unused icon part.
Updated•4 years ago
|
Comment 63•4 years ago
|
||
Dropped the 32 image size since the icon is only ever displayed as 16px by 16px.
Comment 64•4 years ago
|
||
Comment 65•4 years ago
|
||
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.
Comment 66•4 years ago
|
||
Comment 67•4 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/ab0cf15447b2
Replace xul:image usage in menulist-editable. r=mkmelin
Updated•4 years ago
|
Comment 68•4 years ago
|
||
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
Comment 69•4 years ago
|
||
All the textual information in the icon tooltip is already available in the status box adjacent to it.
Comment 70•4 years ago
|
||
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.
Updated•3 years ago
|
Comment 71•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1c0c6a085c8f
Remove alt text that duplicates the title. r=mkmelin
Updated•3 years ago
|
Comment 72•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/668a02c0a6db
Remove title from chat-conversation-info status icon. r=freaktechnik
Updated•3 years ago
|
Comment 73•3 years ago
|
||
Comment 74•3 years ago
|
||
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
Comment 75•3 years ago
|
||
(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.)
Comment 76•3 years ago
|
||
(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.
Reporter | ||
Comment 77•3 years ago
|
||
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.
Comment 78•3 years ago
|
||
(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"
?
Reporter | ||
Comment 79•3 years ago
|
||
Well, and AUDIO. But otherwise correct.
Comment 80•3 years ago
|
||
(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.
Comment 81•3 years ago
|
||
(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".
Comment 82•3 years ago
|
||
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.
Comment 83•3 years ago
|
||
Comment 84•3 years ago
|
||
Updated•3 years ago
|
Comment 85•3 years ago
|
||
Comment 86•3 years ago
|
||
Updated•3 years ago
|
Comment 87•3 years ago
|
||
Comment 88•3 years ago
|
||
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
Comment 89•3 years ago
|
||
Updated•3 years ago
|
Comment 90•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/aa4a4b2932ac
Replace xul:image usage in openpgp key generation/import. r=mkmelin
Comment 91•3 years ago
|
||
Updated•3 years ago
|
Comment 92•3 years ago
|
||
Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/d392076e308e
Replace xul:image usage in sanitation dialog. r=mkmelin
Comment 93•3 years ago
|
||
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.
Comment 94•3 years ago
|
||
Comment 95•3 years ago
|
||
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.
Comment 96•3 years ago
|
||
(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.
Comment 97•3 years ago
|
||
Comment on attachment 9231185 [details]
Bug 1683865 - Replace "add recipient field" xul elements with html buttons. r=aleca
Revision D119897 was moved to bug 1721352. Setting attachment 9231185 [details] to obsolete.
Updated•3 years ago
|
Comment 98•3 years ago
|
||
Also supported tab shifting to the status-bar through F6.
Comment 99•3 years ago
|
||
Also updated the content policy to allow moz-icon image sources.
Also combined the behaviour for img elements whose src values are allowed to be invalid into ImgUtils.jsm.
Comment 100•3 years ago
|
||
Comment 101•3 years ago
|
||
Comment 102•3 years ago
|
||
Comment 103•3 years ago
|
||
Comment 104•3 years ago
|
||
Updated•3 years ago
|
Comment 105•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/18c1f76631f6
Replace xul:image usage in cloud provider selection dialog. r=aleca
https://hg.mozilla.org/comm-central/rev/991627ce316e
Changed status bar to HTML. r=mkmelin
https://hg.mozilla.org/comm-central/rev/b23d8206a1e9
Make the popup browser url headers look like the content tab header. r=mkmelin
https://hg.mozilla.org/comm-central/rev/5a1b41edf4fd
Replace xul:image usage in preferences application icons. r=mkmelin
https://hg.mozilla.org/comm-central/rev/3dfb738fa834
Replace xul:image usage in message-bar-icon. r=mkmelin
Comment 106•3 years ago
|
||
Comment 107•3 years ago
|
||
Comment 108•3 years ago
|
||
Also, previously the image was missing because the info-icon-telemetry image (taken from mozilla-central) had no corresponding CSS in scope.
Comment 109•3 years ago
|
||
Comment 110•3 years ago
|
||
Comment 111•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 112•3 years ago
|
||
Comment 113•3 years ago
|
||
Comment on attachment 9235628 [details]
Bug 1683865 - Replace junkMailInfo dialog with a link to the junk support page. r=mkmelin
Revision D122258 was moved to bug 1725220. Setting attachment 9235628 [details] to obsolete.
Comment 114•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4383c12604c0
Drop the empty no-results-image. r=mkmelin
https://hg.mozilla.org/comm-central/rev/05eeba0d2703
Replace xul:image usage in telemetry preferences. r=mkmelin
https://hg.mozilla.org/comm-central/rev/886ae0b97414
Replace xul:image usage in newmailalert. r=mkmelin
https://hg.mozilla.org/comm-central/rev/d1210f26e9cb
Replace xul:image usage in create CardDAV dialog. r=mkmelin
Comment 115•3 years ago
|
||
Updated•3 years ago
|
Comment 116•3 years ago
|
||
The images were always empty (no src or list-style-image) and took up no space.
Comment 117•3 years ago
|
||
Comment 118•3 years ago
|
||
Updated•3 years ago
|
Comment 119•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/6fabadfb4f73
Clean out activity manager. r=aleca
https://hg.mozilla.org/comm-central/rev/f6d936ae7135
Replace xul:image usage in mail storage converter dialog. r=mkmelin
https://hg.mozilla.org/comm-central/rev/7aeafac65b60
Remove unused xul:image elements in gloda autocomplete richlistitems. r=mkmelin
https://hg.mozilla.org/comm-central/rev/daa47b19e997
Replace xul:image usage in search boxes. r=mkmelin
Updated•3 years ago
|
Comment 120•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/f96c863c78cf
Replace xul:image usage in chat badge button. r=mkmelin
Reporter | ||
Comment 121•3 years ago
|
||
(In reply to Pulsebot from comment #119)
https://hg.mozilla.org/comm-central/rev/daa47b19e997
Replace xul:image usage in search boxes. r=mkmelin
The quick filter search box used to get a "clear" icon on the right after you type something. That's not appearing anymore.
Comment 122•3 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #121)
The quick filter search box used to get a "clear" icon on the right after you type something. That's not appearing anymore.
That search box is controlled by mozilla-central search-textbox
. The regression is caused by https://hg.mozilla.org/mozilla-central/rev/64cf5966f871
Comment 123•3 years ago
|
||
Updated•3 years ago
|
Comment 124•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/c1c11744674a
Replace xul:image usage for updates. r=mkmelin
Comment 125•3 years ago
|
||
The original definition that zeroed the padding, margin and border-radius ended up being overwritten in most use cases.
Comment 126•3 years ago
|
||
Also got rid of some unnecessarily precise ... > .tab-stack > ...
parts of CSS selectors.
Depends on D123432
Reporter | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 127•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/7f8909debcf4
Adjust the icon-button class to plain-button. r=mkmelin
https://hg.mozilla.org/comm-central/rev/c6695c5b5708
Replace xul:image usage in tab close icon. r=mkmelin
Updated•3 years ago
|
Comment 128•3 years ago
|
||
Updated•3 years ago
|
Comment 129•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/3fef60eeb2d0
Replace xul:image usage in tab (fav)icon. r=mkmelin
Comment 130•3 years ago
|
||
Status update
Here are the known remaining locations within comm-central that still contain a xul:image, and why they won't be changed yet.
- mailnews/compose/content/askSendFormat.xhtml displays a platform dependant message/question/warning icon - This dialog should be removed with Bug 1727493.
- mail/extensions/openpgp/content/ui/enigmailMsgBox.xhtml displays a platform dependant message/question/warning/error icon - This dialog should be removed with Bug 1716759.
- mail/base/content/messenger.xhtml uses
#addons-notification-icon
and#app-update-notification-icon
- The code for this is a bit messy and controlled byExtensionsUI.jsm
andGlobalPopupNotifications.jsm
, but these are both based onExtensionsUI.jsm
andPopupNotifications.jsm
from mozilla-central. We can wait and see how this is handled there. - mail/base/content/profileDowngrade.xhtml uses an info icon - Based on mozilla-central file of the same name.
- mail/components/addrbook/content/abMailListDialog.xhtml, abEditListDialog.xhtml, abCard.inc.xhtml, addressbook.xhtml - Old address book is being replaced with Bug 1705276.
- extensionControlled.js close icon is a xul:image - This section was introduced in Bug 1711274 and is ported from mozilla-central, but seems to be broken anyway because we are missing fluent entries and the icon sources. See Bug 1728323.
- mail/components/customizableui/content/customizeMode.inc.xhtml
panel-arrow
icon (which I think is empty anyway) - Based on mozilla-central.
Note that xul:images still exist in other places in thunderbird, but only from code in mozilla-central, such as the icons used in xul menus and buttons.
Comment 131•3 years ago
|
||
Added an alt attribute. Also removed the tooltiptext attribute which is incorrect for html elements; whilst it could have been replaced with the title attribute, the text is already shown below the input, so there is no need to duplicate it.
Comment 132•3 years ago
|
||
Updated•3 years ago
|
Comment 133•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/013d34bdb558
Follow up to 32100bb1fe5f that fixes the fingerWarning icon. r=freaktechnik
https://hg.mozilla.org/comm-central/rev/5cb8f8479f80
Add alt attribute that was missing in rev e5f34a08a2d5. r=darktrojan
Updated•3 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 134•2 years ago
|
||
Context Update
There are two different tags in the code base:
<image
- Searchfox results
<xul:image
- Searchfox results
Edit: Shared the above information for context and as a follow-up to comment #0. comment #135 provides further context.
Comment 135•2 years ago
|
||
(In reply to Elizabeth Mitchell from comment #134)
There are two different tags in the code base:
<image
- Searchfox results
<xul:image
- Searchfox results
Yes, they're basically the same. The xul
prefix is used when the context of the parent element is pure HTML.
Assignee | ||
Comment 136•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Comment 137•2 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/afd690ad68cf
Replace app notifications icons. r=aleca
Comment 138•2 years ago
|
||
(In reply to Pulsebot from comment #137)
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/afd690ad68cf
Replace app notifications icons. r=aleca
Can the list-style-images https://searchfox.org/comm-central/rev/1749f88c7a80c1a2b5bafc95111a82736175cb18/mail/themes/shared/mail/messenger.css#822 and https://searchfox.org/comm-central/rev/1749f88c7a80c1a2b5bafc95111a82736175cb18/mail/themes/shared/mail/messenger.css#861 be removed?
Comment 139•2 years ago
|
||
Oh yes, it seems we can remove those attributes.
I guess we could also remove the addons-icon
class entirely form that element since it's not used.
Assignee | ||
Comment 140•2 years ago
|
||
I will remove those attributes. Thank you! Will look into removing the addons-icon
class as well.
Assignee | ||
Comment 141•2 years ago
|
||
- Remove .addons-icon class
- Remove list-style-image attribute from .app-update-icon class
- Refactor is needed as a result of changes in:
https://hg.mozilla.org/comm-central/rev/afd690ad68cf
Assignee | ||
Updated•2 years ago
|
Comment 142•2 years ago
|
||
Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/34a26d8261e0
Remove unused CSS from app notification icons. r=aleca
Assignee | ||
Comment 143•2 years ago
|
||
Status Update
Remaining instances:
As of today, there are 8 combined remaining instances in the code.
Reporter | ||
Comment 144•2 years ago
|
||
For the case in enigmailMsgBox.xhtml, that whole file and its consumers should be ripped out and replaced with standard prompts.
Assignee | ||
Comment 145•2 years ago
|
||
- Refactor icon to use img instead of xul:image
- Replace icon in new list dialog and edit list dialog
Assignee | ||
Comment 146•2 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #144)
For the case in enigmailMsgBox.xhtml, that whole file and its consumers should be ripped out and replaced with standard prompts.
Thanks for sharing this information. I found more information about enigmailMsgBox.xhtml in comment #130 and Bug 1716759. I'll touch base with Alessandro Castellani [:aleca] about this.
Comment 147•2 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #144)
For the case in enigmailMsgBox.xhtml, that whole file and its consumers should be ripped out and replaced with standard prompts.
Yup, we will tackle this in bug 1716759, so we can close this metabug once the other images are replaced.
Assignee | ||
Updated•2 years ago
|
Comment 148•2 years ago
|
||
Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/feb4c44c3f24
Replace person icon in address book list dialogs. r=aleca
Assignee | ||
Comment 149•2 years ago
|
||
- Replace xul:image icon with html:img icon
- Previous icon was not loading in the dialog
Assignee | ||
Comment 150•2 years ago
|
||
- Replace xul:image with img icon
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 151•2 years ago
|
||
Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/9416876e8afe
Replace icon in profile downgrade dialog. r=aleca
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 152•2 years ago
|
||
From this work, you can also ignore the images in customizeMode.inc.xhtml
, since we're rebuilding the customization mode for the toolbar from scratch in pure HTML, we will soon delete all those files, so no need to update them.
So the last piece of work needed to close this are the 2 images in panelUI.inc.xhtml
.
Assignee | ||
Comment 153•2 years ago
|
||
Okay. Thanks for the update.
Comment 154•2 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/ccb7ec2faf62
Update icon in common dialog. r=aleca
Assignee | ||
Comment 155•2 years ago
|
||
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Comment 156•2 years ago
|
||
I think we can finally close this meta bug after this last patch lands.
We can tackle the enigmail dialog in its dedicated bug.
Assignee | ||
Comment 157•2 years ago
|
||
The enigmail dialog will be handled outside of this [meta] bug.
Comment 158•2 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/806a5fdd8543
Replace icons in panelUI. r=aleca
Reporter | ||
Updated•2 years ago
|
Description
•