1910090 - [macOS] Fandom Facebook sharing popup window extends to the bottom of the screen

Status: RESOLVED WORKSFORME

(Core :: DOM: Window and Location, defect)

Description

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Found in

• Firefox 128.0.3

Affected versions

• Firefox 128.0.3
• Firefox 129.0b8
• Firefox 130.0a1

Tested platforms

• Affected platforms: macOS 13.6.7
• Unaffected platforms: Ubuntu 24.04, Windows 11

Steps to reproduce

1. Launch Firefox and go to an article from Fandom
2. Select Facebook sharing option
3. Observe the new window's behavior

Expected result

• The new window has a fixed height

Actual result

• The new window will slowly extend until it reaches the bottom end of the screen

Regression range

• Will see for a regression

Additional notes

• The issue can be seen in the following attachment

Comment 1

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

I have performed the regression range using Mozregression and the following results were obtained:

• Last good build: 2024-06-04
• First bad build: 2024-06-05
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b0e051062dfb4131e037e1c034353df66cb3d607&tochange=f42a7bd094a97ad88e1d8467c11292361e777422
• Potentially regressed by: bug 1681457

I am removing the "regressionwindow-wanted" keyword.

Comment 2

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

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

:nika, since you are the author of the regressor, bug 1681457, could you take a look?

For more information, please visit BugBot documentation.

Comment 3

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

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

Comment 4

[Nika Layzell [:nika] (ooo, ni? for response)]

It appears that the Facebook page is trying to resize the pop-up window constantly causing it to grow. This was previously blocked as we would mark the window as non-resizeable prior to bug 1681457, but it was made able to be resized in that bug as it is a pop-up window which was presumably created with noopener.

Parsing through obfuscated code in the debugger suggests that it is doing roughly the following while the window is at-rest:

setInterval(() => {
  var winsize = {
    width: window.innerWidth, // W
    height: window.innerHeight, // H
  };
  var docsize = {
    width: document.scrollingElement.scrollWidth, // W
    height: document.scrollingElement.scrollHeight, // H + 1
  };
  var hdiff = docsize.height - winsize.height; // 1
  var wdiff = docsize.width - winsize.width; // 0
  if (hdiff < 0) hdiff = 0;
  if (wdiff <= 1) wdiff = 0;
  if (hdiff || wdiff) {
    window.resizeBy(wdiff, hdiff);
    if (wdiff) window.moveBy(wdiff / -2, 0);
  }
}, 100);

In Firefox, document.scrollingElement.scrollHeight appears to consistently return a height 1 pixel taller than window.innerHeight. This in turn means that every time this 100ms setInterval fires, the window is resized to be 1px taller.

As some additional weirdness, the window appears to also freak out a bit when resized, especially horizontally, perhaps in part due to a desync between these two values being read every 100ms.

We could turn off the ability for windows like this one which are opened by a website with noopener set to resize themselves again, which would appear to fix the issue, but perhaps it would be better to fix this by addressing the discrepancy which is leading to the odd resizing behaviour.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

So this is because the real window size ends up being fractional (e.g. 925.5). .innerHeight truncates (see bug 1889244 and related issues, otherwise websites get confused in other interesting ways), but .scrollHeight rounds here.

So this is somewhat annoying because other browsers afaict also behave this way IIRC. So maybe we should try not to end up with a fractional viewport size?

That said, probably we should contact facebook and ask them to do in the vertical axis the same thing they do in the horizontal one, where they seem to be dealing with it:

  if (hdiff < 0) hdiff = 0;
  if (wdiff <= 1) wdiff = 0;

Note there's https://github.com/w3c/csswg-drafts/issues/5260 to provide reasonable APIs at least for the viewport side of things...

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Dennis seems we could contact Facebook to suggest something like that? The original code looks more like this:

      function i() {
        var a = c('getViewportDimensions') (),
        b = j(),
        e = c('Layer').getTopmostLayer();
        if (e) {
          e = (e = e.getRoot()) == null ? void 0 : e.firstChild;
          e ||
          d('FlowMigrationUtilsForLegacyFiles').invariantViolation('topMostLayer.getRoot().firstChild is null.');
          var f = d('DOMDimensions').getElementDimensions(e);
          f.height += d('DOMDimensions').measureElementBox(e, 'height', !0, !0, !0);
          f.width += d('DOMDimensions').measureElementBox(e, 'width', !0, !0, !0);
          b.height = Math.max(b.height, f.height);
          b.width = Math.max(b.width, f.width)
        }
        e = b.height - a.height;
        f = b.width - a.width;
        f < 0 &&
        c('isFalsey') (h.widthElement) &&
        (f = 0);
        f = f > 1 ? f : 0;
        c('isFalsey') (h.allowShrink) &&
        e < 0 &&
        (e = 0);
        if (e || f) try {
          window.console &&
          window.console.firebug,
          window.resizeBy(f, e),
          f &&
          window.moveBy(f / - 2, 0)
        } catch (a) {
        }
      }

But it's the difference between e and f what's interesting / relevant here. The relevant mailing list is giving me 500 errors now...

Comment 7

[Dennis Schubert [:denschub]]

This no longer seems to be an issue - I see the window resize once, but it resizes into a reasonable size. Catalin, can you confirm this is fixed for you, too?

Comment 8

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Hey Dennis, looked over Firefox 134.0b3 and Firefox 135.0a1 and it seems that this is no longer reproducible for me either.

Comment 9

[Gregory Pappas [:gregp]]

Should this bug be closed?

Comment 10

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Yes, will close it. Thanks!