Closed Bug 1625805 Opened 4 years ago Closed 4 years ago

Don't look at image state for content: url() images.

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(1 file)

This is the root cause of bug https://github.com/webcompat/web-bugs/issues/50775 and I think it should work.

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.

(Even if they belong to an <img> element).

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?)

(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...)

Flags: needinfo?(emilio)

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.

Flags: needinfo?(emilio)

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 this content styling should have the same effect on these elements (whatever the effect is)

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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: