1936259 - yandex.com - Images are not loaded when accessed from image search

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect, P1)

Description

[Raul Bucata]

Environment:
Operating system: Windows 11
Firefox version: Firefox 135.0

Steps to reproduce:

1. Navigate to: https://yandex.com/images/search?img_url=https%3A%2F%2Fdiscovery55.ru%2Fwp-content%2Fuploads%2F2016%2F03%2F36542_original.jpg&lr=87&nl=1&pos=2&rpt=simage&source=morda&text=Altai
2. Open an image from the list of image searches and observe

Expected Behavior:
The image loads

Actual Behavior:
The image does not load

Notes:

• Reproduces regardless of the status of ETP
• Reproduces in firefox-nightly
• Does not reproduce in firefox-release, and chrome
• Sometimes navigation through the image slider is needed to replicate the issue

Created from https://github.com/webcompat/web-bugs/issues/144934

Comment 1

[Raul Bucata]

Attachment: release vs nightly

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a4434b7aabdac9a19413cbeff360c874bb138110&tochange=7df628ab162c87151d7783027905fa7950da76ef

Work around:
.MMImage-Origin {
opacity: 1 !important;
}

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1926015

:TYLin, since you are the author of the regressor, bug 1926015, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Alice0775 White from comment #2)

Work around:
.MMImage-Origin {
opacity: 1 !important;
}

This happens to be a workaround but it's not quite hitting the right element. In e.g. Chrome, this .MMImage-Origin element has opacity:0 as well, so that's not actually the element that's supposed to paint here.

Really, the site has two copies of the img for some reason -- this .MMImage-Origin one that Alice0775's workaround found (which is supposed to be opacity:0 and hence hidden), and another copy with class="MMImage-Preview" which is supposed to show up.

It's the MMImage-Preview one that we're sizing to be 0x0 for some reason now (and hence not painting).

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1 (from testcase-reducer)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2 (using lime canvas instead of img, to avoid large image download)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 3 (expected to render a wide lime area)

Comment 8

[Daniel Holbert [:dholbert]]

I'm morphing this into a regularly-classified regression bug rather than a webcompat site-report, because this was an actual regression that we'd ideally like to avoid shipping rather than a webcompat bug that's downstream of some known/longstanding platform issue.

Comment 9

[Jonathan Kew [:jfkthame]]

In testcase 3 (comment 7), I notice that the issue only happens when both width and height properties are specified as percentages on the lime canvas. If either of those is removed, or changed to specify some other units (px, em, vh, whatever...) it works as expected -- and the percentage in the other axis then works, too.

Comment 10

[Jens Stutte [:jstutte]]

[Tracking Requested - why for this release]: The severity is high, though it derives from the webcompat report, maybe for a platform bug it is too high?

Comment 11

[Daniel Holbert [:dholbert]]

I think it's accurate to consider it high-severity; and we're pretty sure this the same as bug 1934719. TYLin is investigating.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox134 (beta). However, the bug still isn't assigned.

:fgriffith, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 13

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I'm looking into this.

Comment 14

[Frank Griffith Jr]

Thanks Ting-Yu!

Comment 15

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1936259 Part 1 - Make Set() and Get() in IntrinsicISizesCache consistent in the logic. r?#layout

Use "percentage" in the variable name since other member variables use it.
Introduce usePercentageAwareCache in Get() to ensure consistency with
Set().

Comment 16

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1936259 Part 2 - Clear mCacheWithPercentageBasis whenever mLastPercentageBasis is changed. r?#layout

In Get(), when the percentage-aware cache is used, we always reject the cache
whenever the percentage basis changes. Therefore, in Set() we should clear
mCacheWithPercentageBasis after mLastPercentageBasis is assigned to a new
value. Otherwise, if the out-of-line cache is already allocated, not clearing
mCacheWithPercentageBasis can result in a stale PrefISize cache after
MinISize cache is updated, and vice versa.

I didn't add a new wpt based on the reduced testcase 3 in the bug because I
failed to reduced it to a simple enough state that I can describe it clearly.
However, this patch fixed two existing WPTs that have a dynamic update to an
absolute element containing a aspect-ratio child. I feel it's enough for
covering this patch.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5159e4a66b19
Part 1 - Make Set() and Get() in IntrinsicISizesCache consistent in the logic. r=dshin
https://hg.mozilla.org/integration/autoland/rev/e8772f6333f5
Part 2 - Clear mCacheWithPercentageBasis whenever mLastPercentageBasis is changed. r=dshin,dholbert

Comment 19

[amarc]

https://hg.mozilla.org/mozilla-central/rev/5159e4a66b19
https://hg.mozilla.org/mozilla-central/rev/e8772f6333f5

Comment 20

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 9444189 [details]
Bug 1936259 Part 1 - Make Set() and Get() in IntrinsicISizesCache consistent in the logic. r?#layout

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: No user impact because this doesn't change behavior.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch doesn't change behavior. It is a preparation for Bug 1936259 Part 2.
• String changes made/needed: None.
• Is Android affected?: Yes

Comment 21

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 9444190 [details]
Bug 1936259 Part 2 - Clear mCacheWithPercentageBasis whenever mLastPercentageBasis is changed. r?#layout

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: The images on yandex.com (this bug) and Kanopy.com (Bug 1934719) are missing or distorted.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch invalidate a stale size cache, and it doesn't change how the size is computed.
• String changes made/needed: None.
• Is Android affected?: Yes

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9444189 [details]
Bug 1936259 Part 1 - Make Set() and Get() in IntrinsicISizesCache consistent in the logic. r?#layout

Approved for 134 RC1.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/1e109259b418
https://hg.mozilla.org/releases/mozilla-beta/rev/e8eb4c4815d4

Comment 24

[Raul Bucata]

Verified as fixed on the latest Firefox RC Build 134.0. Images are loading when accessed from an image search. Navigation through the image slider also loads the images as expected.

Tested with:

Browser / Version: Firefox 134.0 Candidate
Operating System: Windows 10 PRO x64
Operating System: Ubuntu 22.4 LTS x64
Operating System: Mac Ventura 13.1