Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diydrones.com - fails inserting new content correctly on scrolling down due to outdated jQuery #412

Closed
webcompat-bot opened this issue Nov 2, 2014 · 10 comments
Milestone

Comments

@webcompat-bot
Copy link

URL: http://diydrones.com/m?id=705844%3AMobilePage%3A857813
Browser / Version: Firefox Mobile 36.0
Operating System: Android
Problem type: Looks like the website has a bug.
Site owner: No

Steps to Reproduce

  1. Navigate to: http://diydrones.com/m?id=705844%3AMobilePage%3A857813
  2. scroll down

Expected Behavior:display new blog entries
Actual Behavior:displays last blig entry over and over

@hallvors
Copy link

hallvors commented Nov 3, 2014

Whenever you scroll to the bottom of the page, the page starts loading new content. However, it ends up adding the same entry again and again!

Loading data, this code is involved:
x$(_d.html).hide().appendTo(_a).trigger('create').fadeIn('slow');

_d.html is the new list of articles from the server. A bit further down the stack, we hit this code:

this.each(function () {
        var b = p.queue(this, a, c);
        p._queueHooks(this, a),
        a === 'fx' && b[0] !== 'inprogress' && p.dequeue(this, a)
      })
    },

The each command will loop through all nodes in a DOM fragment - including text nodes. And if a text node a is passed to this code:

  function bZ(a, b) {
    return a = b || a,
    p.css(a, 'display') === 'none' || !p.contains(a.ownerDocument, a)
  }

it will throw because p.css() relies on getComputedStyle() which doesn't expect to be used with text nodes. I've verified that this error causes the problem.

I'm not sure why this doesn't occur in other browsers. Perhaps the script uses a DOM parser that creates text nodes for whitespace in Firefox and uses a different approach in other browsers?

@hallvors
Copy link

hallvors commented Nov 3, 2014

Actually, getComputedStyle() doesn't throw in Chrome even if you pass a text node to it. That's why the failure occurs only in Firefox.

@hallvors
Copy link

hallvors commented Nov 3, 2014

@hallvors
Copy link

hallvors commented Nov 4, 2014

This is basically failing in jQuery code

g = a.length;
for (; f < g; f++) {
  c = a[f];
  if (!c.style) continue; // this line fixes the problem
  e[f] = p._data(c, 'olddisplay'),
  b ? (!e[f] && c.style.display === 'none' && (c.style.display = ''), c.style.display === '' && bZ(c) && (e[f] = p._data(c, 'olddisplay', cc(c.nodeName))))  : (d = bH(c, 'display'), !e[f] && d !== 'none' && p._data(c, 'olddisplay', d))
}

Which means their jQuery library is outdated since jQuery on GitHub has exactly the same fix I added above - here:
https://github.com/jquery/jquery/blob/740e190223d19a114d5373758127285d14d6b71e/src/css.js#L161

We should suggest that they update jQuery.

@hallvors hallvors changed the title diydrones.com - Doesnt display desktop version. Doesn't update bottom blog properly diydrones.com - fails inserting new content correctly on scrolling down due to outdated jQuery Nov 4, 2014
@hallvors
Copy link

Bugs to make sure other browsers follow the spec too:
https://bugs.webkit.org/show_bug.cgi?id=138584
https://code.google.com/p/chromium/issues/detail?id=431915

@karlcow
Copy link
Member

karlcow commented Sep 23, 2015

This has been fixed in Chrome Blink and fails in the same way, also in Edge.

This is not fixed in Safari WebKit, which is behind the curve.

The site is created by http://diydrones.com/profile/zlitezlite
https://about.me/andersonchris

The good news is that he is on github @zlite
and might help to fix his site.

@karlcow
Copy link
Member

karlcow commented Feb 19, 2016

@billbonney could you help us find the right person for this issue at @diydrones thanks

@billbonney
Copy link

@karlcow diydrones.com is hosted on a platform supplied by ning.com platform. I'd recommend taking with them

@karlcow
Copy link
Member

karlcow commented Feb 19, 2016

Thanks @billbonney
So if I understand correctly, the platform ning is doing the front-end too.
I wonder if @pierre has still contacts at @ning who could help us.

@karlcow
Copy link
Member

karlcow commented Aug 2, 2016

This has been fixed.

@karlcow karlcow closed this as completed Aug 2, 2016
@karlcow karlcow added this to the fixed milestone Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants