1571756 - Menu on website gets into a resize loop

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect, P1)

Description

[Gian-Carlo Pascutto [:gcp]]

macOS 14.10, current Nightly

Load: https://www.bbc.com/worklife/article/20190801-the-trick-that-makes-you-overspend

Resize browser a bit so the first row (the menu) just doesn't fit. The site starts shimmering as it enters an eternal resizing loop.

dom.block_reload_from_resize_event_handler has no effect.

Didn't observe this behavior in Chome.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

If that pref has no effect, then it's not it. Also it seems there are no reloads involved, just either a layout bug or the site JS doing something weird.

That being said, it'd be nice to know if this is a regression.

Comment 2

[Jens Hausdorf :jens1o [ni? me]]

Ah, I'm sorry :emilio! Just saw it in Slack and I wasn't aware that the Blocks status depends whether it is still affected by the pref or not.

Comment 3

[Karl Dubost💡 :karlcow]

fwiw this is happening in Firefox 68.0.1 (64-bit) on macOS too.

Comment 4

[Karl Dubost💡 :karlcow]

A performance profile
https://perfht.ml/2GQQwDz

I had a narrow window, then expanded, then narrowed again. The janking effect on the menu is happening only when the switch to the new layout is happening. then stops right away when we come back to the initial layout.

Maybe related
https://www.bbc.com/worklife/assets/static/js/bundle.581dde53.js
as it seems to happen when there is a change of mediaqueries

                                                  f = function () {
                                                    function e(e, t, n) {
                                                      var r = this;
                                                      this.nativeMediaQueryList = e.matchMedia(t),
                                                      this.active = !0,
                                                      this.cancellableListener = function () {
                                                        r.matches = r.nativeMediaQueryList.matches,
                                                        r.active && n.apply(void 0, arguments)
                                                      },
                                                      this.nativeMediaQueryList.addListener(this.cancellableListener),
                                                      this.matches = this.nativeMediaQueryList.matches
                                                    }
                                                    return e.prototype.cancel = function () {
                                                      this.active = !1,
                                                      this.nativeMediaQueryList.removeListener(this.cancellableListener)
                                                    },
                                                    e
                                                  }(),

When this is called.

                                                      this.cancellableListener = function () {
                                                        r.matches = r.nativeMediaQueryList.matches,
                                                        r.active && n.apply(void 0, arguments)
                                                      },

r.nativeMediaqueryList.matches is called and this seems to happen only in between (r.nativeMediaqueryList.media )

(min-width: 600px) and (max-width: 1279px)
and
(min-width: 600px) and (max-width: 1007px)

Let's log r.nativeMediaqueryList.media from the max window to min window.

08:38:22.368 (min-width: 600px) and (max-width: 1279px) bundle.581dde53.js:formatted:33705
08:38:22.373 (min-width: 1008px) and (max-width: 1279px) bundle.581dde53.js:formatted:33705

// happening in this range only.

08:38:55.630 (min-width: 600px) and (max-width: 1007px) bundle.581dde53.js:formatted:33705
08:38:55.756 (min-width: 1008px) 2 bundle.581dde53.js:formatted:33705
08:38:55.759 (max-width: 1007px) bundle.581dde53.js:formatted:33705
08:38:55.762 (min-width: 1008px) and (max-width: 1279px) bundle.581dde53.js:formatted:33705
08:38:55.764 (min-width: 768px) and (max-width: 1007px) bundle.581dde53.js:formatted:33705
08:39:29.249 (min-width: 768px) 2 bundle.581dde53.js:formatted:33705
08:39:29.252 (max-width: 767px) bundle.581dde53.js:formatted:33705
08:39:29.253 (min-width: 600px) and (max-width: 767px) bundle.581dde53.js:formatted:33705
08:39:29.254 (max-width: 767px) 2 bundle.581dde53.js:formatted:33705
08:39:29.256 (min-width: 768px) bundle.581dde53.js:formatted:33705
08:39:29.258 (min-width: 768px) and (max-width: 1007px) bundle.581dde53.js:formatted:33705
08:39:29.259 (max-width: 767px) bundle.581dde53.js:formatted:33705
08:39:33.754 (max-width: 599px) bundle.581dde53.js:formatted:33705
08:39:33.755 (min-width: 600px) and (max-width: 1007px) bundle.581dde53.js:formatted:33705
08:39:33.756 (min-width: 600px) and (max-width: 767px) bundle.581dde53.js:formatted:33705
08:39:33.757 (min-width: 600px) and (max-width: 1279px) bundle.581dde53.js:formatted:33705
08:39:37.980 (min-width: 300px) and (max-width: 399px) 2 bundle.581dde53.js:formatted:33705

Comment 5

[Karl Dubost💡 :karlcow]

btw this stops as soon as i set

/* Inline #5 | https://www.bbc.com/worklife/article/20190801-the-trick-that-makes-you-overspend */

.nav-build-bar__links {
  min-width: 0;
}

That might be interesting for Daniel.

Comment 6

[Alice0775 White]

I can reproduce the issue on Windows10 Nightly70.0a1.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=805f1a2737a0ef58a64cf8e7b6239543d4b45940&tochange=dc6c04a63309d3461c0ca9dc23ae186a91deb8c6

Triggered by:
dc6c04a63309d3461c0ca9dc23ae186a91deb8c6 Robert Longson — Bug 1396642 - support smaller viewBox coordinates at the expense of larger ones r=dholbert

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Following up on n-i in email.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this a valid regression or is it a site issue?

Comment 9

[Daniel Holbert [:dholbert]]

Sorry, I missed the needinfo. Is this still reproducible? I'm having trouble reproducing it. If so and if Bug 1396642 is really the commit blamed via mozregression, I'm guessing it was just unlucky, and the site could already have gotten into this state if it used slightly different markup.

If it is reproducible, could someone attach a screenshot and/or screencast of the Firefox window when the bad thing is happening? That would help me know how wide to make the window when trying to test. Comment 0 says "Resize browser a bit so the first row (the menu) just doesn't fit", but this is hard to do because the site is very responsive and goes through a series of different versions of that menu with progressively less + smaller content so that it always fits. :) I'm not sure which transition-to-a-smaller-layout is the sometimes-problematic one.

Comment 10

[Alice0775 White]

Attachment: screencast

I can reproduce the issue on Nightly71.0a1.

Build ID 20190917093629
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Comment 11

[Alice0775 White]

FYI, In my case,
If the following svg viewBox properties was changed to "0 0 107.9 16" from "0 0 107.9 15.8", then the issue was gone.

<svg version="1.1" id="Layer_1" xmlns="https://www.w3.org/2000/svg" xmlns:xlink="https://www.w3.org/1999/xlink" x="0px" y="0px" viewBox="0 0 107.9 15.8" style="enable-background:new 0 0 107.9 15.8;" xml:space="preserve">

...

Comment 12

[Daniel Holbert [:dholbert]]

Odd... I still can't reproduce on Linux Nightly 2019-09-17. Tested on my desktop (1:1 DPI) and my laptop (HiDPI, tested with 200% and 300% scaling).

In comment 10, the threshold seems to be the point where "Bright Sparks" appears/disappears (to the right of "Worklife 101"). I was not able to reproduce any flickering when resizing my window around that threshold.

A few thoughts:

• This might be font-dependent (which could explain why I haven't been able to repro on Linux)
• There seems to be a bit of "fudge factor" in their decision of when to show/hide the Bright Sparks text. In Responsive Design Mode, starting with a wide viewport, the "Bright Sparks" is visible with viewport-width=896px and disappears when I drop to 895px -- but then if I go back up to 896px, it doesn't reappear until I get to 897px.

Anyway -- comment 11 is very helpful in explaining how Bug 1396642 may have impacted things here. Before Bug 1396642, we probably rounded the given viewBox values (0 107.9 15.8) to 0 0 108 16 under the hood. Bug 1396642's change in behavior (supporting finer-grained values) will have made the sizing calculations more precise & slightly different here. So whatever measurement BBC's JS is doing to make the call about whether to show or hide this content, it will be getting slightly different measurements now.

And to be clear, the content is hiding/showing due to JS that the site is running -- they actually move the element back and forth between different places in the DOM entirely. They hide it by moving it to a different container with class=nav-build-bar__links-hidden which is display:none -- that's how it disappears. So I think this is just a WebCompat issue, and likely a bug in the site's JS.

Comment 13

[Daniel Holbert [:dholbert]]

(In reply to Karl Dubost💡 :karlcow from comment #5)

btw this stops as soon as i set

/* Inline #5 | https://www.bbc.com/worklife/article/20190801-the-trick-that-makes-you-overspend */

.nav-build-bar__links {
  min-width: 0;
}

That might be interesting for Daniel.

This doesn't actually fix the issue, I don't think. It just prevents them from ever hiding "Bright Sparks" at all (for me at least) -- it ends up overflowing and overlapping the hamburger menu etc. as you shrink the page size. It eventually disappears after a media query is triggered (I think), but not until much later than their dynamic-hiding magic would like for it to be hidden.

Comment 14

[Daniel Holbert [:dholbert]]

Shot in the dark: they might have a rounding error in whatever JS math that they use when they're estimating how many elements will fit into the available space. They probably detect overflow, and then remove the "Bright Sparks" element (for example) and then re-run their JS to reevaluate-how many elements will fit, and then they mistakenly think the removed element would fit without overflowing, and they add back the removed element, and repeat.

Comment 15

[Mike Taylor [:miketaylr]]

Ksenia, can you try to dig in here?

Comment 16

[Gian-Carlo Pascutto [:gcp]]

Odd... I still can't reproduce on Linux Nightly 2019-09-17.

I can't either (on Linux), but it still reproduces on an older Macbook Pro (which has twice the resolution!). Is there something I can check for you?

Comment 17

[Ksenia Berezina [:ksenia]]

This is the function that is causing the loop:

{
  key: "updateDimensions",
  value:
function () {
            var e = this,
                t = this.navBar.current;
            if (t) {
                var n = this.navBarLinks.current,
                    r = this.navBarOpenSponsored.current,
                    i = this.navBarHideLinks.current,
                    a = this.navBarBranding.current,
                    l = t.offsetWidth,
                    s = this.state,
                    u = s.firstSetup,
                    c = s.sizeOfOldElements,
                    f = 0;
                if (u) {
                    for (this.setState({
                        firstSetup: !1,
                        sizeOfOldElements: []
                    }); i.firstChild;) i.removeChild(i.firstChild);
                    f = 15000
                }
                var d = n.offsetWidth + r.offsetWidth + a.offsetWidth + f;
                if (d > l || this.props.mobile) for (; d > l && n.lastChild && null !== n.lastChild && void 0 !== n.lastChild || this.props.mobile && n.lastChild && null !== n.lastChild && void 0 !== n.lastChild;) !function () {
                    var t = n.lastChild;
                    e.setState(function (e) {
                        return {
                            sizeOfOldElements: [].concat(o(e.sizeOfOldElements), [
                                t.offsetWidth
                            ])
                        }
                    }),
                        d -= t.offsetWidth,
                        i.insertBefore(t, i.firstChild)
                }();
                else null !== i.firstChild && d + c[c.length - 1] <= l && (this.setState(function (e) {
                    var t = e.sizeOfOldElements.slice();
                    return t.pop(),
                        {
                            sizeOfOldElements: t
                        }
                }), null !== i.firstChild && i.firstChild && void 0 !== i.firstChild && n.appendChild(i.firstChild))
            }
        }
}

where "Bright Sparks" link offsetWidth is stored in a component state and then the element is being inserted to a hidden div ( so "Bright Sparks" disappears):

var t = n.lastChild;
        e.setState(function (e) {
                return {
                     sizeOfOldElements: [].concat(o(e.sizeOfOldElements), [
                           t.offsetWidth
                     ])
                }
         }),
        d -= t.offsetWidth,
       i.insertBefore(t, i.firstChild)

since there is a change to a state, componentDidUpdate is triggered:

key: "componentDidUpdate",
           value: function() {
                  setTimeout(this.handleResize, 0)
            }
},
{
key: "handleResize",
         value: function() {
               this.updateDimensions()
         }
 }

And updateDimensions function runs again, but this time this condition is being executed:

else null !== i.firstChild && d + c[c.length - 1] <= l && (this.setState(function (e) {
                    var t = e.sizeOfOldElements.slice();
                    return t.pop(),
                        {
                            sizeOfOldElements: t
                        }
                }), null !== i.firstChild && i.firstChild && void 0 !== i.firstChild && n.appendChild(i.firstChild))

so here "Bright Sparks" is being inserted back to navigation links and this.setState is being run again, triggering the loop over and over.

This is not happening in Chrome, though, so I've compared the values. d + c[c.length - 1] is 945 in Chrome and 944 in Firefox, while l (the wrapping nav bar innerWidth) is 944.
While d (the sum of innerWidths of logo, links and menu elements) is the same in both browsers, c (the innerWidth of hidden "Bright Sparks" ) is different. It's 124.5 in Chrome, they round it to 125 and it's 124.467 in Firefox, we round it to 124. Perhaps, the site shouldn't rely on innerWidth here. My guess is to use getBoundingClientRect().width instead.

As for the regression, the menu innerWidth (svg) has changed as Daniel mentioned in comment 12 - it was smaller before the regression, so I assume (the debbugger doesn't want to work in that version of FF) that in this case the condition if (d > l || this.props.mobile) wasn't satisfied and the first setState never got to run. As a result, no infinite loops :)

UPDATE: contacted the site via mailing list

Comment 18

[Ksenia Berezina [:ksenia]]

Looks like this was fixed