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

Expanding a thread sometimes causes scroll to the top #115

Open
electroly opened this Issue Jan 10, 2019 · 16 comments

Comments

Projects
None yet
3 participants
@electroly
Copy link
Member

electroly commented Jan 10, 2019

https://www.shacknews.com/chatty?id=38235563#item_38235563

I have verified 100% that the page is fully loaded. This isn't the same issue where Shack itself causes a scroll to the top when you expand a post before the page fully loads. Something else is going on.

@wjrogers

This comment has been minimized.

Copy link
Collaborator

wjrogers commented Jan 10, 2019

This has been happening to me intermittently, too, and it's super annoying. Like just close the tab and do something else annoying. Agree it's not the old "page wasn't fully loaded" shack bug.

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 10, 2019

I'll investigate this afternoon. I have some ideas as to what might be causing it, but I need to find more reproducible examples of it.

One possiblity is that page event handlers may be getting disrupted by one of Chrome Shack's scripts.

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 10, 2019

Which browsers and versions has this been observed to affect? Chrome, if so, what version; Firefox, if so, what version? Other webkit based browsers?

@wjrogers

This comment has been minimized.

Copy link
Collaborator

wjrogers commented Jan 10, 2019

I'm on Chrome Version 71.0.3578.98 (Official Build) (64-bit)

@electroly

This comment has been minimized.

Copy link
Member

electroly commented Jan 10, 2019

Exact same version of Chrome here.

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 10, 2019

I think I have a list of steps to reproduce this on both Firefox and Chrome:

  • Enable the 'Infinite Scrolling' script
  • Scroll to the second page of the Chatty so that elder_scroll.js processes the new root post node
  • Click any fullpost or a child of a fullpost on this extended root post node (a post that would be on the n-th page)
  • Observe that the native page jQuery does not know how to locate this fullpost node so it scrolls to the top and stays there

To be clear, this is not necessarily a Chrome Shack problem, as it stems from the native scripts on the Chatty page not knowing how to handle new fullpost nodes tacked onto the root post container. I think the solution to this is to override the page scroll behavior when the 'Infinite Scrolling' script is enabled to workaround this behavior. Essentially extending a special-case of the Auto-post Focus feature to enable the expected behavior.

Also, crxMouse gestures not working when the 'Performance Hack' script is enabled is a different issue tied to blocking mousemove events at the window level. Simply moving this to the document level should fix the issue.

I'll try to get both of these fixes out as soon as I can.

@electroly

This comment has been minimized.

Copy link
Member

electroly commented Jan 10, 2019

Data point: I don't use the Infinite Scrolling script. This is happening with that script off.

@electroly

This comment has been minimized.

Copy link
Member

electroly commented Jan 10, 2019

Also it happens to threads on the first page. Ain't nobody looking at the second page.

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 10, 2019

I'm not sure why you'd be seeing the behavior with 'Infinite Scrolling' turned off. Although I do notice that clickItem() in chatview.min.js on Chatty is throwing exceptions for some threads when it fires after a click. An exception could be halting execution of another dependent function. It seems to be a visibility filter, and given the scrollTo() call it's using, there's a good chance that has to do with the behavior you're seeing:

function clickItem(b, f) {
    var d = window.frames.dom_iframe;
    var e = d.document.getElementById("item_" + f);
    if (uncap_thread(b)) {
        elem_position = $("#item_" + f).position();
        window.scrollTo(0, elem_position.top - scroll_y + 12)
    }
    sLastClickedItem = f;
    sLastClickedRoot = b;
    if (d.document.getElementById("items_complete") && e) {
        var c = find_element(e, "DIV", "fullpost");
        var a = import_node(document, c);
        show_item_fullpost(b, f, a);
        return false
    } else {
        path_pieces = document.location.pathname.split("?");
        parent_url = path_pieces[0];
        navigate_page_no_history(d, "/frame_chatty.x?root=" + b + "&id=" + f + "&parent_url=" + parent_url);
        return false
    }
}
@electroly

This comment has been minimized.

Copy link
Member

electroly commented Jan 10, 2019

I can reproduce in Version 73.0.3667.0 (Official Build) canary (32-bit).

I couldn't get it to happen with the default out-of-box settings. Then I enabled all the Chrome Shack scripts (but NOT Infinite Scrolling) and it happened immediately the first time I clicked on a thread on the first page. (edited)

I doubt it matters but this is the post I clicked on: https://www.shacknews.com/chatty?id=38236784#item_38236784

@wjrogers

This comment has been minimized.

Copy link
Collaborator

wjrogers commented Jan 11, 2019

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 12, 2019

Hmm, so I take it that this behavior is seen when the "Auto post focusing" option is disabled? If so, I may have a way around it: I could leverage the methods used to support the carousel (and in the future the lightbox) to re-run an injected replacement for clickItem() (seen above) to force the post to be focused when 'uncapping' a thread.

Speaking of which, I need a little clarification on the behavior that's being reported here. If a fullpost is 'capped', and the user clicks a reply within the fullpost, the post will 'uncap' and the window will scroll to somewhere near the top of that uncapped post? If that's an relatively accurate set of steps, I may have a way to reproduce this behavior for testing pretty easily.

@wjrogers

This comment has been minimized.

Copy link
Collaborator

wjrogers commented Jan 12, 2019

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 13, 2019

Well, I'm stumped. After trying for 3 days I can't reproduce this behavior on any of my machines with any version of Firefox or Chrome I've tested. It sounds like a script is throwing an exception somewhere.

I'll have some fixes up soon for various exceptions I've found while testing for this issue.

(Update: I think I may have found a way to reproduce it in my test environment by using an old Linux laptop with a 3rd gen i5 in it. I have no idea why the processor type/revision matters in this case, but I can't reproduce this on more modern machines that I own.

It does seem related to the clickItem() function being busted on the Chatty itself, but I'm still in the process of nailing down why Chrome Shack seems to trigger that behavior. I'll probably have to inject a custom clickItem() function to override the page and correct the misbehaving code.)

@wjrogers

This comment has been minimized.

Copy link
Collaborator

wjrogers commented Jan 15, 2019

I just captured this exception in the debugger after I started getting the scroll-to-top bug every time I expanded a post. Seems like it might be a clue. I don't really know what I'm doing.

image

@WombatFromHell

This comment has been minimized.

Copy link
Contributor

WombatFromHell commented Jan 15, 2019

The bizarre thing about that screenshot is that the exception you're seeing should never happen when simply clicking a post - only when clicking the 'refresh' icon of a post. Aside from that, nothing shown there helps explain the "scroll to top" issue. I do appreciate the debugger screenshot though.

I've not been able to duplicate the "scroll to top" problem on every click of a post, but I have been able to duplicate it when uncapping a collapsed fullpost. I'm convinced that at this point it would be a good idea to remove our "auto-focus post" logic from Chrome Shack, and work toward monkey patching or overriding the busted scroll logic on the Chatty page itself because that's ultimately the root of the problem that Chrome Shack is trying to solve by forcefully scrolling to what the user clicks on.

TL;DR: I'm working on it. I apologize for how annoying this issue is for some users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment