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

Comment timestamp element with timestampId: CommentTopMeta--Created--abc123 not found - onScroll event fires before onUrlChange #1

Open
nemanjam opened this issue Dec 11, 2023 · 0 comments

Comments

@nemanjam
Copy link
Owner

nemanjam commented Dec 11, 2023

I have documented it with comments in this commit:

390e8ac

This exception is handled but nevertheless it would be good to prevent it from happening at all. It happens sometimes when you navigate back from Reddit thread back to subreddit page.

It gets caught here in handleScrollDom():

export const handleScrollDom = async () => {

export const handleScrollDom = async () => {
  if (!isActiveTabAndRedditThread()) return;

  const commentElements = document.querySelectorAll<HTMLElement>(commentSelector);
  if (!(commentElements.length > 0)) return;

  try {
    await highlightByDateWithSettingsData(commentElements); // this line throws it

    await delayExecution(markAsRead, markAsReadDelay, commentElements);
    if (!isActiveTabAndRedditThread()) return;

    await highlight(commentElements);
  } catch (error) {
    logger.error('Error handling comments onScroll:', error);
  }
};

It originates from here:

const getDateFromCommentId = (commentId: string): Date => {

const getTimestampIdFromCommentId = (commentId: string) => {
  const modalSuffix = hasModalScrollContainer() ? timestampIdModalSuffix : ''; //! this failed to detect overlay
  const timestampId = timestampIdPrefix + commentId + modalSuffix;
  return timestampId;
};

const getDateFromCommentId = (commentId: string): Date => {
  const timestampId = getTimestampIdFromCommentId(commentId); // this returns timestampId without 'inOverlay' suffix so element can't be found
  const timestampElement = document.querySelector<HTMLElement>(`#${timestampId}`);

  if (!timestampElement)
    throw new MyElementNotFoundDOMException(
      `Comment timestamp element with timestampId: ${timestampId} not found.`
    );

  const timeAgo = timestampElement.textContent as string;

  const date = relativeTimeStringToDate(timeAgo);
  return date;
};

// this function fails to detect overlay because onScroll fired before onUrlChange 
// and onScroll has 1 second debounce delay which isn't enough for DOM to load
export const hasModalScrollContainer = (): boolean => {
  const modalScrollContainer = document.querySelector<HTMLElement>(
    modalScrollContainerSelector
  );
  return Boolean(modalScrollContainer);
};

Main point: onScroll fires before onUrlChange and onScroll has scrollDebounceWait of 1 seconds which isn't enough for DOM to load compared to urlChangeDebounceWait which waits for 2 seconds. Because of that overlay isn't detected and it generates comment timestampId without inOverlay suffix. Then that element isn't found and it throws an exception.

export const scrollDebounceWait = 1000;

This happens because Reddit has SPA where url change doesn't really load new DOM, it just handles url and rerenders part of the existing DOM and the existing onScroll and onUrlChange event handlers remain attached. In handleUrlChange() I remove onScroll event handler but that doesn't seem enough to prevent onScroll from firing before onUrlChange.

There should be some better synchronization mechanism for which I don't yet have solution, maybe some global lock variables but it doesn't sound right.

const handleUrlChange = async (previousUrl: string, currentUrl: string) => {

const handleUrlChange = async (previousUrl: string, currentUrl: string) => {
  // modal or document
  const scrollElement = getScrollElement();

  if (hasArrivedToRedditThread(previousUrl, currentUrl)) {
    scrollElement.addEventListener('scroll', debouncedScrollHandler);

    // listen keys on document
    document.addEventListener('keydown', handleCtrlSpaceKeyDown);

    // test onUrlChange and onScroll independently
    await handleUrlChangeDom();
  }

  if (hasLeftRedditThread(previousUrl, currentUrl)) {
    scrollElement.removeEventListener('scroll', debouncedScrollHandler);
  }
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant