webcompat / web-bugs

A place to report bugs on websites.
https://webcompat.com
Mozilla Public License 2.0
741 stars 63 forks source link

www.goal.com - Disqus comments are no displayed #87227

Open webcompat-bot opened 3 years ago

webcompat-bot commented 3 years ago

URL: https://www.goal.com/en-in/news/guardiola-refuses-to-apologise-after-clarifying-comments/hnbn987quhvd1xfxfllvpz1w6

Browser / Version: Firefox 94.0 Operating System: Linux Tested Another Browser: Yes Chrome

Problem type: Something else Description: Disqus comments not appearing Steps to Reproduce:

  1. load page
  2. scroll down what happens:

Disqus comments don't appear.

View the screenshot Screenshot
Browser Configuration
  • gfx.webrender.all: false
  • gfx.webrender.blob-images: true
  • gfx.webrender.enabled: false
  • image.mem.shared: true
  • buildID: 20210917161824
  • channel: nightly-autoland
  • hasTouchScreen: true
  • mixed active content blocked: false
  • mixed passive content blocked: false
  • tracking content blocked: false

View console log messages

From webcompat.com with ❤️

softvision-oana-arbuzov commented 3 years ago

Thanks for the report, I was able to reproduce the issue. image

Note:

  1. This might be related to #47666
  2. The issue is not reproducible on Chrome.
  3. The issue is reproducible on Firefox Nightly regardless ot the ETP status.

Tested with: Browser / Version: Firefox Nightly 94.0a1 (2021-09-19) Operating System: Windows 10 Pro

Moving to Needsdiagnosis for further investigation.

softvision-raul-bucata commented 2 years ago

The issue is also reproducible using an Android mobile device

Tested with: Browser / Version: Firefox Release 94.1.2 (2015844587-🦎94.0-20211028161635🦎)/ Firefox Nightly 96.0a1 (2015845995 -🦎96.0a1-20211115093917🦎) Operating System: Samsung A51 (Android 11) -1080 × 2400 pixels 20:9 aspect ratio (~405 ppi density)

Moving this to NeedsDiagnosis for further investigations.

denschub commented 2 years ago

They ship a completely different layout to Firefox, so there's a lot going on here. But they do that based on UA sniffing. As soon as the UA has the word "Chrome" in it, everything works, even a UA like

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:96.0) Gecko/20100101 Firefox/96.0 Chrome-for-WebCompat

Let's snip an intervention and reach out...

denschub commented 2 years ago

Actually, let's close as a duplicate of #47666 - which this is.

softvision-oana-arbuzov commented 2 years ago

Based on the comment https://github.com/webcompat/web-bugs/issues/47666#issuecomment-1028078553 , the design indeed has changed and it is no longer broken, but the issue related to "Disqus comments not being displayed" is still reproducible.

Tested with: Browser / Version: Firefox Nightly 99.0a1 (2022-02-07) Operating System: Windows 10 Pro

Reopening this issue and moving to "Needsdiagnosis" for further investigation on the new layout. @denschub can you have a look over this? Should we enable back the intervention https://bugzilla.mozilla.org/show_bug.cgi?id=1741892?

[inv_06/2022]

karlcow commented 2 years ago

The comments are opening for me. Firefox Nightly 99 on macOS Capture d’écran 2022-02-16 à 16 22 26

softvision-oana-arbuzov commented 2 years ago

Indeed, the comments load now, regardless if I enable or disable the goal.com UA Override. image

Tested with: Browser / Version: Firefox Nightly 99.0a1 (2022-02-15) Operating System: Windows 10 Pro

@RandomDust01 can you still reproduce it on your side with the latest Firefox version?

[inv_07/2022]

RandomDust01 commented 2 years ago

@softvision-oana-arbuzov I tested with Firefox on windows 10. Comments are loading without any issues. But the problem has been with Firefox on Android. Comments still not loading on goal.com

Tested with Firefox 97.2.0 on Android

softvision-oana-arbuzov commented 2 years ago

Thanks @RandomDust01. The initial issue was reported on desktop layout, which seems to be working now. On mobile layout the "Comments" section does not load at all, while on Chrome it loads. image

Tested with: Browser / Version: Firefox Nightly 99.0a1 (🦎 99.0a1-20220216094238), Firefox Release 97.1.0 (🦎 97.0-20220131171509) Operating System: Samsung Galaxy S8 (Android 9) - 1440 x 2960 pixels, 18.5:9 ratio (~570 ppi density)

I'll reopen it.

RandomDust01 commented 2 years ago

@softvision-oana-arbuzov thanks. This has been an issue with Android Firefox for over six months as far as I can remember. Comments section loads without any issue on older Android Firefox 68.0. It does not load on any of the newer versions as far as I am aware. Thought I should bring it to your attention

karlcow commented 2 years ago
<div class="article-content_thread__2w2-Q">
  <div class="disqus-thread">
    <div id="disqus_thread"></div>
  </div>
</div>

it calls

  t = function () {
    return a.getComputedStyle ? function (b, c, d) {
      try {
        return a.document.defaultView.getComputedStyle(b, null).getPropertyValue(c)
      } catch (e) {
        return null
      }
    }
     : function (a, b, c) {
      return a.currentStyle[b] || a.currentStyle[c]
    }
  }()

which returns undefined.

because b is { margin: "", padding: "", border: "", fontSize: "0px" }

And we get an error on Uncaught TypeError: can't access property "split", c is undefined

when hitting

    function l(a) {
      for (var b, c = k(a, 'span', 'font-family', 'fontFamily'), d = c.split(','), e = {
        courier: 1,
        times: 1,
        'times new roman': 1,
        georgia: 1,
        palatino: 1,
        serif: 1
      }, f = 0; f < d.length; f++) if (b = h(d[f]), e.hasOwnProperty(b)) return !0;
      return !1
    }

fontFamily seems to be always undefined and it fails without fallback.

@jfkthame do you know if there was a change with font treatment in between 68 and later versions?

jfkthame commented 2 years ago

I don't recall any changes that would affect how the properties are exposed to JS.

Regarding the Uncaught TypeError: can't access property "split", c is undefined, that would appear to point to the k function having failed to do....whatever it was supposed to?

karlcow commented 2 years ago

Note that there is probably code path involved, because if I fake the UA to be Safari on iOS, this is working.

Yes I have forgotten to paste the function k.

    function k(a, b, c, d) {
     // u(b) checks if b is a string `[object String]`
     // in this case b == `span`
     // b becomes an HTMLElement
      u(b) && (b = z.createElement(b));
      var e = null;
     // sets the visibility to hidden
      return b.style.visibility = 'hidden',
     // a is `<div id="disqus_thread"></div>`
     // so it becomes `<div id="disqus_thread"><span style="visibility: hidden;"></span></div>`
      a.appendChild(b),
     // 
      e = j(b, c, d),
      a.removeChild(b),
      e
    }
    // a is <span>
    // b is "font-family"
    // c is "fontFamily"
    function j(a, b, c) {
     // c will be 'fontFamily'
      c = c || b;
      // d is `undefined` here in fact it is calling `t` below.
      var d = w(a, b, c);
      //
      return !d || /color/i.test(b) && 0 === x(d).alpha ? a && j(a.parentNode, b, c) || d : d || null
    }
  t = function () {
    return a.getComputedStyle ? function (b, c, d) {
      try {
       // b is `<span style="visibility: hidden;">`
       // c is `fontFamily`
       // a.document.defaultView.getComputedStyle(b, null) returns 
       // {margin: "", padding: "", border: "", fontSize: "0px" }
       // which in the end returns getPropertyValue being undefined.
        return a.document.defaultView.getComputedStyle(b, null).getPropertyValue(c)
      } catch (e) {
        return null
      }
    }
     : function (a, b, c) {
      return a.currentStyle[b] || a.currentStyle[c]
    }
  }()

So that's it. The getComputedStyle returns a different beast here in Blink and Gecko.

Capture d’écran 2022-02-21 à 12 56 07

a.document.defaultView.getComputedStyle(b, null) returns a CSSStyleDeclaration and window.document.defaultView.getComputedStyle(b, null).getPropertyValue('fontFamily') returns an empty string.

This is calling a polyfill for getComputedStyle. https://polyfill.app/api/polyfill?minify&features=scroll-behavior,intl|locale=en,intersection-observer

/**
 * Polyfills applied (in order): performance.now, request-animation-frame, scroll-behavior, window, get-computed-style, event.focusin, event.hashchange, intersection-observer
 * @preserve
 */

Their getComputedStyle from the polyfill code calls

    function I(s) {
      var h = this,
// currentStyle ???
      d = s.currentStyle,
      T = y(s, 'fontSize'),
      _ = function (L) {
        return '-' + L.toLowerCase()
      },
      v;
      for (v in d) if (Array.prototype.push.call(h, v == 'styleFloat' ? 'float' : v.replace(/[A-Z]/, _)), v == 'width') h[v] = s.offsetWidth + 'px';
       else if (v == 'height') h[v] = s.offsetHeight + 'px';
       else if (v == 'styleFloat') h.float = d[v];
       else if (/margin.|padding.|border.+W/.test(v) && h[v] != 'auto') h[v] = Math.round(y(s, v, T)) + 'px';
       else if (/^outline/.test(v)) try {
        h[v] = d[v]
      } catch (L) {
        h.outlineColor = d.color,
        h.outlineStyle = h.outlineStyle || 'none',
        h.outlineWidth = h.outlineWidth || '0px',
        h.outline = [
          h.outlineColor,
          h.outlineWidth,
          h.outlineStyle
        ].join(' ')
      } else h[v] = d[v];
      g(h, 'margin'),
      g(h, 'padding'),
      g(h, 'border'),
      h.fontSize = Math.round(T) + 'px'
    }

https://github.com/financial-times/polyfill-service

ok.

https://github.com/Financial-Times/polyfill-service/blob/673a28d7ee06458cf26ea7e0f2fc08a7fee4fb68/source/window.getComputedStyle.js#L5-L7

This is calling currentStyle

So yes I'm not sure why they did that, but they are calling the polyfill for getComputedStyle in case of Firefox on Android. That's a site mistake.

webcompat-bot commented 2 years ago

Generate outreach template

karlcow commented 2 years ago

according to https://github.com/Financial-Times/polyfill-service/tree/0507744420c6007d9d822ee134f594b0530053e6/polyfills/Window.prototype.getComputedStyle

it should not even go there, but still the getComputedStyle is called thousand of times. ???

karlcow commented 2 years ago

it looks a bit like this issue https://github.com/Financial-Times/polyfill-service/issues/1138 misuse of the polyfill service by goal

I sent them an email.

karlcow commented 2 years ago

I was wondering why https://polyfill.app/api/polyfill?minify&features=scroll-behavior,intl|locale=en,intersection-observer

seems to react differently depending on the UA string.

Firefox Android UA

GET /api/polyfill?minify&features=scroll-behavior,intl%7Clocale=en,intersection-observer HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: polyfill.app
User-Agent: Mozilla/5.0 (Android 12; Mobile; rv:98.0) Gecko/98.0 Firefox/98.0
HTTP/1.1 200 OK
Access-Control-Allow-Headers: Content-Type,Device-Language,Accept,Authorization,Version,Pragma
Access-Control-Allow-Methods: GET,PUT,POST,DELETE,OPTIONS
Access-Control-Allow-Origin: *
Cache-Control: public,max-age=31536000,immutable
Connection: keep-alive
Content-Encoding: gzip
Content-Length: 8146
Content-Type: application/javascript; charset=utf-8
Date: Tue, 22 Feb 2022 00:25:38 GMT
ETag: ddcea6a2b5a83d125b27dea8585f09f4a2d3cea4
Server: nginx/1.18.0 (Ubuntu)
X-Powered-By: Express
x-applied-polyfills: performance.now, request-animation-frame, scroll-behavior, window, get-computed-style, event.focusin, event.hashchange, intersection-observer

iOS Safari UA

GET /api/polyfill?minify&features=scroll-behavior,intl%7Clocale=en,intersection-observer HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: polyfill.app
User-Agent: Mozilla/5.0 (iPhone; CPU iPhone OS 14_6 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.3 Mobile/15E148 Safari/604.1
HTTP/1.1 200 OK
Access-Control-Allow-Headers: Content-Type,Device-Language,Accept,Authorization,Version,Pragma
Access-Control-Allow-Methods: GET,PUT,POST,DELETE,OPTIONS
Access-Control-Allow-Origin: *
Cache-Control: public,max-age=31536000,immutable
Connection: keep-alive
Content-Encoding: gzip
Content-Length: 42166
Content-Type: application/javascript; charset=utf-8
Date: Tue, 22 Feb 2022 00:26:41 GMT
ETag: 70ae127e12e11310433f6cdc099fcef24c1200a5
Server: nginx/1.18.0 (Ubuntu)
X-Powered-By: Express
x-applied-polyfills: scroll-behavior, intl.display-names (locale: en), intl.list-format (locale: en), intl.number-format (locale: en)

user agent sniffing on the server side for polyfill.app

and while safari iOS doesn't receive a polyfilled getComputedStyle(), Firefox Android does… ??? for no obvious reasons.

So probably the one to contact are polyfill.

The app is hosted at https://github.com/wessberg/polyfiller by @wessberg