1584641 - Investigate replacing <xul:image> with a new element using `content: <url>` instead of `list-style-image`, and `object-[position|fit]` instead of -moz-image-region

Status: ASSIGNED

(Toolkit :: UI Widgets, task)

Description

[Brian Grinstead [:bgrins]]

I don't know if this would fully replace the functionality. See https://bugzilla.mozilla.org/show_bug.cgi?id=1403231#c2 for some background here.

We could also support the [src] attribute / property using a custom element that then sets the relevant CSS properties on itself.

If the basic mechanism (content instead of list-style-image) works, then a couple options to look at:

• This could be a XUL element if we wanted to be able to use [flex] etc, or we could try making it an HTML element.
• We could continue to use xul:image for now and rewrite CSS to use the new properties.

Comment 1

[Tim Nguyen :ntim]

One thing worth noting is that list-style-image inherits to all its children, but only has an effect on actual XUL images, whereas content works directly on the element it's being applied on.

For instance:

<box style="list-style-image: url(foo)">
  <image id="1"/>
  <image id="2"/>
  <image id="3"/>
</box>

list-style-image applies to all 3 XUL images in this case, but has no effect on the box itself.

<div style="content: url(http://placekitten.com/300/300)">
  <img id="1" src="http://placekitten.com/300/200"/>
  <img id="2" src="http://placekitten.com/300/400"/>
  <img id="3" src="http://placekitten.com/300/500"/>
</div>

The content on div will essentially replace all 3 children in this case.

It should still be possible however to replace list-style-image with content by being more careful about the selectors being used (eg. setting content on #back-button > .toolbarbutton-icon as opposed to just #back-button, and similarly elsewhere...).

Comment 2

[Tim Nguyen :ntim]

Also, regarding -moz-image-region: rect(), this function takes 4 values: top, right, bottom, left. object-position on the other hand works more like background-position, it's still possible to reproduce the same results using object-fit+width/height however, but it requires a bit of re-writing.

There are only 150 usages of -moz-image-region:
https://searchfox.org/mozilla-central/search?q=-moz-image-region&case=false&regexp=false&path=

If you take out -moz-image-region: auto which essentially resets to the default behaviour (which we shouldn't need to do assuming we've dealt with the inheritance of list-style-image), there are only 47 hits for -moz-image-region: rect(:
https://searchfox.org/mozilla-central/search?q=-moz-image-region%3A+rect(&case=false&regexp=false&path=

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1584641 - Investigation - can we use an html element with content: <url> instead of <xul:image>?

It looks pretty possible. We could also start doing this piecemeal, letting the two elements live alongside each other for a while. There are some todos:

• Finish finding and changing styles to target directly the moz-image elements instead of relying on inheriting styles (this is done in a proof of concept here
  to render the main toolbarbuttons we see at startup. This patch has some logging to try to help with that. Mozscreenshots would come in handy too.
• Handle moz-image-rect as ntim outlines in https://bugzilla.mozilla.org/show_bug.cgi?id=1584641#c2
• .tabbrowser-arrowscrollbox::part(scrollbutton-down) is used to set list-style-image. We want to dig into the content of that node, but can't with Shadow Parts.
  We'd need export parts or something similar for that (Bug 1559076)

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #3)

• .tabbrowser-arrowscrollbox::part(scrollbutton-down) is used to set list-style-image. We want to dig into the content of that node, but can't with Shadow Parts.
  We'd need export parts or something similar for that (Bug 1559076)

Tentatively marking as depends on Bug 1559076 because of this. I'm sure we could work around this in the frontend (CSS variable set on host, for instance).

Comment 5

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #4)

Tentatively marking as depends on Bug 1559076 because of this. I'm sure we could work around this in the frontend (CSS variable set on host, for instance).

As I mentioned on Slack, a workaround could simply be using background-image for this specific case.

Also, I'm marking as blocking bug 1590179 because this would allow ripping out nsImageBoxFrame.

Comment 7

[Tim Nguyen :ntim]

Looking at -moz-image-region, there aren't many usages left, which can be categorized as such:

1. panelUI.inc.css uses them to resize addon icons and display them correctly I think. The sync usage doesn't look useful since the icon used there has the same size already. The addon icon usage can use <html:img> + object-fit + width/height, or background-image/size + width/height

2. notification-icons.inc.css & infobar.inc.css use them for translation icons. Fwiw, keeping the colored version and applying a grayscale could also just work, or alternatively, just remove this code?

3. browser/themes/windows/browser.css uses it for Windows 7 window controls. Windows 10 doesn't use it. Could do something like 1., but probably not worth it.

4. toolkit/themes/windows/global/global.css uses it for the old print preview UI. This should be removed soon-ish anyway.

5. pageInfo.css uses them for header icons on Linux/Windows. The icon sprites can each be split into 3 files, or could be replaced by SVG.
   Both Linux & Windows have an unused feed icon.
   Windows icons have a hover/selected variant that doesn't look different from the normal one (see pageInfo.png). Even if they are different, a filter would do the job.

6. all the auto values can be removed when the rect() usages above will be removed (since auto is only used to reset rect())

7. remaining are test usages

Over time, they should just fade out from the codebase, but removing -moz-image-region is already totally feasible now if wanted (this could be a mentored task even).
Fwiw, platform support in nsTreeBodyFrame.cpp can already be removed, since nothing uses in trees (maybe TB?).

This bug should probably focus more on handling inheritance and any behaviour specific to the XUL image layout frame.

Comment 8

[Magnus Melin [:mkmelin]]

Are there reasons not to simply use <html:img src="chrome://..... ex.svg"> for cases where it's just the one <xul:image>?

Comment 9

[Magnus Melin [:mkmelin]]

<html:img> can't use content: url(); at least not yet - bug 1484928.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

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

<html:img> can't use content: url(); at least not yet - bug 1484928.

You could pretty much use <html:div> with content: url() right? What am I missing?

Comment 11

[Magnus Melin [:mkmelin]]

Correct, it just wasn't obvious plain <img> wouldn't work for cases where different themes use different image mappings.
Semantically it seems wrong to use something other than <img>, but on the technical level it may not matter much.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

<img> should be usable now, see bug 1742411.