1905646 - The shop's thumbnails are not visible on UberEats possibly due to lazy loading behavior difference between browsers

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Tooru Fujisawa [:arai]]

Attachment: shop's thumbnail being invisible

Steps to reproduce:

1. Run Nightly 129.0a1 (2024-06-30) (64-bit) with clean profile on macOS
2. open https://www.ubereats.com/
3. Enter zip code (e.g. "100-0001")
4. click "Find Food"
5. scroll if necessary

Actual result:

Most shop's thumbnails are not visible, but light-gray box is shown.

Expected result:

All shop's thumbnail are shown.

Some observation:

When I inspect the image, "visibility: hidden" style is applied to the image.

Example, copied from DevTools Inspector:

<img
  alt=""
  role="presentation"
  src=""
  srcset="https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/e39556ef1e4c4565bc6c7dbd8b90869c.jpeg 240w,https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/7915c4a78a9f94ed56316c7c4dc0ec89.jpeg 550w,https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/67b1ce06a25a64dc4a71581bb39c36c6.jpeg 640w,https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/f3376a06b92224efbe50167fb7cb61e4.jpeg 750w,https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/50446f64f31cbefe66558fc47f50a9d6.jpeg 1080w,https://tb-static.uber.com/prod/image-proc/processed_images/5a277b76dd53c4c4fcfe1467425a0b4e/c9252e6c6cd289c588c3381bc77b1dfc.jpeg 2880w"
  sizes="25vw"
  loading="lazy"
  decoding="async"
  class="ed ae bh kv i3"
  style="visibility: hidden;">

The style comes from the following code, where it looks like error event is dispatched on the img element.

https://www.ubereats.com/_static/client-main-fc07aee107883a6d.js (prettified)

      return n ? a && a.endsWith('.svg') && (u = 'img') : u = 'presentation',
      (0, o.tZ) ('img', {
        ref: t,
        alt: n,
        role: u,
        src: i() (a),
        onError: t=>{
          e.onError && e.onError(t),
          _('eats:image_load_error', {
            imageSrc: a
          }),
          t.target instanceof HTMLImageElement &&
            (t.target.style.visibility = 'hidden')
        },
        ...s
      })

There i() (a) for the src property is empty string.
When I comment out the src: i() (a), line, the thumnails are shown shown and I don't observe the error event.

The object passed to ...s above contains srcSet property with non-empty string,
so at least both src and srcset are present from the beginning.

(If srcset wasn't present at first, it will try to load src which points the current HTML file and it will dispatch error event due to decoding error or something)

Comment 1

[Tooru Fujisawa [:arai]]

So far, here's what I observed:

The for (u in ve(t, l), s = l) below iterates over the properties from the above,
and the property value flows into b(e, u, c, i):

https://www.ubereats.com/_static/client-vendor-react-57d02f50089116f6.js

    function Bo(e, n, t) {
      var r = n.pendingProps;
      switch (Gl(n), n.tag) {
...
        case 5:
          tu(n);
          var l = Za(Ga.current);
          if (t = n.type, null !== e && null != n.stateNode) Po(e, n, t, r),
          e.ref !== n.ref && (n.flags |= 512, n.flags |= 2097152);
           else {
...
            if (e = Za(Ya.current), oa(n)) {
...
            } else {
...
              e: {
...
                for (u in ve(t, l), s = l) if (s.hasOwnProperty(u)) {
                  var c = s[u];
                  'style' === u
                    ? he(e, c)
                    : 'dangerouslySetInnerHTML' === u
                      ? null != (c = c ? c.__html : void 0) && ce(e, c)
                      : 'children' === u
                        ? 'string' == typeof c
                          ? ('textarea' !== t || '' !== c) && fe(e, c)
                          : 'number' == typeof c && fe(e, '' + c)
                        : 'suppressContentEditableWarning' !== u &&
                          'suppressHydrationWarning' !== u &&
                          'autoFocus' !== u &&
                          (o.hasOwnProperty(u)
                           ? null != c && 'onScroll' === u && Mr('scroll', e)
                           : null != c && b(e, u, c, i))
                }

b(e, u, c, i) performs setAttribute on the img element.
At this point, the img element doesn't have parent node.

https://www.ubereats.com/_static/client-vendor-react-57d02f50089116f6.js

    function b(e, n, t, r) {
...
      }(n) &&
      (null === t
       ? e.removeAttribute(n)
       : e.setAttribute(n, '' + t))
     : l.mustUseProperty
       ? e[l.propertyName] = null === t
         ? 3 !== l.type && ''
         : t
       : (n = l.attributeName,
          r = l.attributeNamespace,
          null === t
          ? e.removeAttribute(n)
          : (t = 3 === (l = l.type) ||
             4 === l &&
             !0 === t
             ? ''
             : '' + t,
             r
             ? e.setAttributeNS(r, n, t)
             : e.setAttribute(n, t))))

After all attributes are populates (including both src and srcset),
the element is somehow connected to the document, and then (or maybe before get connected?) it causes the error event.

Then, after that, yet another setAttribute call happens t times in the following
(at this point, img has parent node)

https://www.ubereats.com/_static/client-vendor-react-57d02f50089116f6.js

    function di(e, n) {
      var t = e.alternate,
      r = e.flags;
      switch (e.tag) {
...
        case 5:
...
          if (4 & r && null != (l = e.stateNode)) {
...
            if (e.updateQueue = null, null !== s) try {
              'input' === i && 'radio' === u.type && null != u.name && G(l, u),
              ye(i, o);
              var c = ye(i, u);
              for (o = 0; o < s.length; o += 2) {
                var f = s[o],
                d = s[o + 1];
                'style' === f
                  ? he(l, d)
                  : 'dangerouslySetInnerHTML' === f
                    ? ce(l, d)
                    : 'children' === f
                      ? fe(l, d)
                      : b(l, f, d, c)

and then load event is dispatched on the img element twice.

There's no difference in the element attributes between:

• after for (u in ve(t, l), s = l) loop, and
• after for (o = 0; o < s.length; o += 2) loop

So at least the difference between error vs load doesn't seem to be caused by the attributes themselves.

Then, one thing I notice is that, the behavior for the following case differs between Firefox vs Chromium/Safari

• img element is not attached to the tree
• img element has src attribute with empty string
• img element has srcset attribute with valid sources
• img element has loading attribute with lazy value

testcase:
data:text/html,<script>const img = document.createElement("img"); img.onload = () => { console.log("load"); }; img.onerror = ( ) => { console.log("error"); }; img.setAttribute("src", ""); img.setAttribute("srcset", "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVQI12NgYPgPAAEDAQDZqt2zAAAAAElFTkSuQmCC 100w, data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVQI12NgYPgPAAEDAQDZqt2zAAAAAElFTkSuQmCC 200w"); img.setAttribute("loading", "lazy");</script>

Firefox shows error, but Chromium/Safari show load.

So, the possibility is that, there's some delay between setting attributes and attaching to the tree, and error event happens only on Firefox, and it applies the visibility: hidden, and then the image loads properly once the img is attached to the tree.

Comment 2

[Tooru Fujisawa [:arai]]

Ikezoe-san, can you take a look?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

The object passed to ...s above contains srcSet property with non-empty string, so at least both src and srcset are present from the beginning.

I think this might be basically bug 1647077, which depends on bug 1076583... It seems the site is setting src, then srcset, then loading="lazy". So we trigger the src load synchronously, which triggers an error and is no good.

Arai, does that sound plausible?

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Yeah, comment 1 is basically bug 1647077.

Comment 5

[Tooru Fujisawa [:arai]]

Yes, reordering the property for o.tZ call in order to make srcset and loading appear before src solves the issue.
Thank you!

Comment 6

[Dennis Schubert [:denschub]]

Reopening this to keep tracking it as a site bug depending on bug 1647077.

Comment 8

[Tooru Fujisawa [:arai]]

This issue persists on the builds after bug 1076583 (I've tried on autoland 28af98ea).
The ubereats website still shows the light-gray box, and also the minimal testcase in comment #1 still logs error.

Perhaps there's yet another issue, around the load timing, or an interaction between empty src vs non-empty srcset ?
It looks like the empty src still wins in the testcase, while non-empty srcset wins in other browsers.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

No. I had to keep .src="" triggering a sync load for compat, see https://github.com/whatwg/html/issues/2429. However that doesn't seem to match what Blink is doing...

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1905646 - Don't trigger src="" load sync if there's no current request. r=smaug

Seems to match what other browsers do. Other browsers seem to fire
events at random some of the time tho, fun stuff as usual.

Comment 11

[Tooru Fujisawa [:arai]]

Thank you for the info and the patch!
With the patch applied onto m-c b7131a95dd254929f23ef852f5e34ae7cd67e37f, I confirmed the fix on the ubereats website and also the testcase.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eacc9aa46ff1
Don't trigger src="" load sync if there's no current request. r=smaug

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47493 for changes under testing/web-platform/tests

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/eacc9aa46ff1

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot