Don't look at image state for content: url() images.
Categories
(Core :: Layout, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox76 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
This is the root cause of bug https://github.com/webcompat/web-bugs/issues/50775 and I think it should work.
Assignee | ||
Comment 1•4 years ago
|
||
Reduced test-case is data:text/html,<img alt="foo" height="30" style="content: url(https://www.w3.org/2008/site/images/logo-w3c-mobile-lg)">
. The alt text causes it to go through the normal display-based code-path. There's no reason to draw alt text there.
Assignee | ||
Comment 2•4 years ago
|
||
(Even if they belong to an <img> element).
Comment 3•4 years ago
|
||
Do we need more bugs/patches/testcases for cases with no alt text?
This bug/patch/testcase seems to be alt-text-specific, but Chrome seems to have roughly the same behavior here even if I remove the alt text, which makes sense to me. E.g. they render these as a native-sized image and a 30x30 image, respectively:
data:text/html,<img style="content: url(https://www.w3.org/2008/site/images/logo-w3c-mobile-lg)">
data:text/html,<img width="30" height="30" style="content: url(https://www.w3.org/2008/site/images/logo-w3c-mobile-lg)">
...whereas Firefox NIghtly render a 0x0 and 30x30 blank box for these cases. Does the patch help with these cases? (& should it?)
Comment 4•4 years ago
|
||
(Put another way: comment 1 seems to be saying that the alt text is breaking stuff here. But AFAICT from my testcases in prev. comment, this seems similarly-broken regardless of whether there's alt text...)
Assignee | ||
Comment 5•4 years ago
|
||
Yes, but those cases without alt
are a bit less clear to me. I filed a bunch of related issues here.
The alt
one it seems it should work: We have alt text, so we fall back through the regular "create a frame based on the style of the element", and it seems to me that if we create an image frame due to content: url()
, it should work there instead of painting the alt text.
However in the two test-cases you post that doesn't seem to be the case. Those are a replaced boxes but due to it being an <img>
, not due to having content
. You can check removing the content: url()
declaration and adding a ::after
rule like: data:text/html,<style>img::after { content: "inline" }</style><img>
. I think Chrome is wrong on those.
So, I think that if <img>
gets a replaced box due to it being an <img>
element, then content
shouldn't work. Otherwise it should not.
Comment 6•4 years ago
|
||
Aha, I see -- so you're saying "content:<image-url>" is expected to work on img-with-alt-text for the same reason that it's expected to work on e.g. span here (and it does work here):
data:text/html,<span style="content: url(https://www.w3.org/2008/site/images/logo-w3c-mobile-lg)">Fail
That makes sense to me, sure.
I think Chrome is wrong on those.
Gotcha. Looking at your github issue, it looks like Tab disagrees, but there's at least agreement on the expectation that content
should have the same effect across replaced elements. (What that effect is is the part that's up in the air - but you & he agree that it's reasonable to expect consistent results.)
Given that.... for completeness, perhaps you could add a variant of your testcase element-replacement-image-alt.html
, with these changes:
- testcase has no alt text
- reference case is the same as the testcase, except with s/img/video/
- testcase maybe has a
<link rel="help">
(or some sort of reference) pointing at your github issue, as a callback to a spot where there's agreement that thiscontent
styling should have the same effect on these elements (whatever the effect is)
Assignee | ||
Comment 7•4 years ago
|
||
I did that, with a few tweaks.
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/43e3d1c09e85 Don't look at image state for content: url() images. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22622 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Comment 11•4 years ago
|
||
bugherder |
Upstream PR merged by moz-wptsync-bot
Description
•