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
Categories
(Toolkit :: XUL Widgets, task)
Tracking
()
People
(Reporter: bgrins, Assigned: bgrins)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
Bug 1584641 - Investigation - can we use an html element with content: <url> instead of <xul:image>?
(deleted),
text/x-phabricator-request
|
Details |
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.
Updated•5 years ago
|
Comment 1•5 years ago
|
||
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•5 years ago
|
||
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®exp=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®exp=false&path=
Assignee | ||
Comment 3•5 years ago
|
||
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)
Assignee | ||
Comment 4•5 years ago
|
||
(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•5 years ago
|
||
(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•4 years ago
|
||
Looking at -moz-image-region
, there aren't many usages left, which can be categorized as such:
-
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
-
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?
-
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.
-
toolkit/themes/windows/global/global.css uses it for the old print preview UI. This should be removed soon-ish anyway.
-
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. -
all the
auto
values can be removed when therect()
usages above will be removed (sinceauto
is only used to resetrect()
) -
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.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 8•4 years ago
|
||
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•4 years ago
|
||
<html:img> can't use content: url();
at least not yet - bug 1484928.
Comment 10•4 years ago
|
||
(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•4 years ago
|
||
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•2 years ago
|
||
<img>
should be usable now, see bug 1742411.
Updated•2 years ago
|
Description
•