XKit-Rewritten icon indicating copy to clipboard operation
XKit-Rewritten copied to clipboard

Correct notification count in tab title when ignoring unread following posts

Open leumasme opened this issue 1 year ago • 9 comments

Concept

The Hide the Home/Following unread badge tweak currently only hides the actual badge via css. This has the downside of leaving the actual browser tab title unaffected, which is constantly updated to (n) Tumblr, where n is the inbox + unread count.
image It would be nice if the Unread count was ignored here too, although this would of course increase the complexity of the patch from a simple css inject.
Since realistically, noone would want the Unread badge hidden but the unread count included in the window title, this should likely not be a separate patch, but an update of the exisitng one.
The easiest way to accomplish both badge hiding and the window title fix is probably patching the unread field in the /counts response to 0. This would also make the badge hiding immune to tumblr css changes.

My apologies if this is what is meant by the "Tab titles" entry in the Feature Progress/unplanned board, but it's pretty ambiguous

leumasme avatar Dec 30 '23 14:12 leumasme

I'd love to have this feature, but last time I attempted to implement it I didn't succeed! If anyone can figure out a way to do it reliably, that would be awesome.

marcustyphoon avatar Dec 30 '23 16:12 marcustyphoon

Is there already any code/utils for intercepting API requests that are sent by Tumblr? If not, this would likely be the place to start (either via the content Script by proxying the fetch/xmlhttprequest calls, or via the extension script by registering an interceptor using browser extension Apis)

leumasme avatar Dec 30 '23 22:12 leumasme

I don't like the idea of modifying Tumblr API responses, especially for something this trivial.

Agree that this tweak should handle the <title> as well, though.

AprilSylph avatar Jan 01 '24 22:01 AprilSylph

What is the argument against patching API calls? Its probably the most robust way to create patches, since the API is generally very unlikely to change unlike e.g. css, js, dom structure etc. You can probably proxy setting the window title too, and then calculate the correct notification count (how? Fetching API yourself? Parsing notification count from html?) and overwrite the window title, but I don't see how anyone could argue that this is simpler than overwriting a json property in the http request.

leumasme avatar Jan 01 '24 23:01 leumasme

I guess it's just that it's uncharted territory. So far, XKit has always built squarely on top of Tumblr, adding our own elements, listeners, even API requests... but patching API responses made by the web app to make it behave differently is quite an alien idea.

Also, it would have to be done via content script, since famously, synchronous webRequest is being removed in Manifest V3, and unfortunately around half of XKit Rewritten users are still using Chrome.

This is all to say that I'm not strictly opposed to it, just wary of it, and not sure if it's necessary to achieve what we want here.

AprilSylph avatar Jan 03 '24 23:01 AprilSylph

Some notes from my attempt at implementation just now:

  1. As far as I can see, the number in the tab title is only the unread post count, not the inbox count as well, so figuring out the correct number should be a non-issue; it should just be removed
  2. Since Tumblr is updating the tab title via React, i.e. by mutating the <title> element in the DOM, it's actually easy to override with our own <title> element prepended to document.head
  3. We unfortunately can't use pageModifications to listen to changes to Tumblr's <title> element (which we want to do to keep our <title> up-to-date with the current page) because pageModifications only listens to <div id="root">
Scrapped code
import { keyToCss } from '../../util/css_map.js';
import { dom } from '../../util/dom.js';
import { buildStyle } from '../../util/interface.js';
import { translate } from '../../util/language_data.js';
import { pageModifications } from '../../util/mutations.js';

const followingHomeButton = `:is(li[title="${translate('Home')}"], button[aria-label="${translate('Home')}"], a[href="/dashboard/following"])`;

const customTitleElement = dom('title', { 'data-xkit': true });
const styleElement = buildStyle(`
${followingHomeButton} ${keyToCss('notificationBadge')} {
  display: none;
}
`);

const onTitleChanged = ([titleElement]) => {
  const rawTitle = titleElement.textContent;
  const newTitle = rawTitle.replace(/^\(\d{1,2}\) /, '');
  customTitleElement.textContent = newTitle;
};

export const main = async () => {
  pageModifications.register('head title:not([data-xkit])', onTitleChanged);
  document.head.prepend(customTitleElement);
  document.documentElement.append(styleElement);
};

export const clean = async () => {
  customTitleElement.remove();
  styleElement.remove();
};

AprilSylph avatar Jan 03 '24 23:01 AprilSylph

For the record, here's my discarded code as of the last commit on the branch:

scrapped code
import { inject } from '../util/inject.js';
import { getPreferences } from '../util/preferences.js';

let observer;
let mode;

const unburyPollerContext = () => {
  const postElement = document.currentScript.parentElement;
  const reactKey = Object.keys(postElement).find(key => key.startsWith('__reactFiber'));
  let fiber = postElement[reactKey];

  while (fiber) {
    const { pollerContext } = fiber.stateNode?.props || {};
    if (pollerContext !== undefined) {
      console.log(fiber);
      return pollerContext;
    } else {
      fiber = fiber.return;
    }
  }
};

const countRegex = /^\(\d+\) /;

const updateTitleCount = async (...args) => {
  console.log('onTitleChanged', args);

  let count = 0;

  if (mode === 'notifications') {
    const {
      notificationCount = 0,
      unopenedGifts = 0,
      unreadMessagesCount = 0,
      unseenPosts = 0
    } = document.querySelector('header')
      ? await inject(unburyPollerContext, [], document.querySelector('header'))
      : {};

    console.log('pollerContext', {
      notificationCount,
      unopenedGifts,
      unreadMessagesCount,
      unseenPosts
    });

    count = notificationCount + unopenedGifts + unreadMessagesCount;
  }

  observer?.disconnect();

  const titleElement = document.head.querySelector('title');
  const currentTitle = titleElement.textContent;
  titleElement.textContent = `${count ? `(${count}) ` : ''}${currentTitle.replace(countRegex, '')}`;

  observer?.observe(titleElement, { characterData: true, subtree: true });
  observer?.observe(document.head, { childList: true });
  delayedUpdateTitleCount();
};

let timeoutID;
const delayedUpdateTitleCount = (...args) => {
  clearTimeout(timeoutID);
  timeoutID = setTimeout(() => updateTitleCount(...args), 35000);
};

export const main = async () => {
  ({ mode } = await getPreferences('title_counts'));
  observer = new MutationObserver(updateTitleCount);
  updateTitleCount();
};

export const clean = async () => {
  clearTimeout(timeoutID);
  observer.disconnect();
  observer = undefined;
};

If I try to recall, I think I had issues with soft navigation and... something else? I think? Not sure what, offhand.

(I, of course, was trying to make it possible for the tab title to include the activity notification count, which is not what this issue is requesting but is a frequently requested feature; that may well have been the part I couldn't make reliable, in which case this would probably work here.)

marcustyphoon avatar Jan 03 '24 23:01 marcustyphoon

(Also for the record, I don't know that I entirely agree with the philosophical argument... or entirely disagree with it either, but I think it's ultimately not super relevant; from a practical perspective, intercepting and modifying API call bodies would be neat but has a fair number of practical pitfalls even beyond the MV3 problem. A bunch of the app state gets set on initial page load/hydration, so you would have to either modify that too or start doing DOM mutations to fix it up on first load anyway, which takes away a lot of the benefit as far as elegance goes.)

marcustyphoon avatar Jan 04 '24 00:01 marcustyphoon

A bunch of the app state gets set on initial page load/hydration

Oh yeah, that would really throw a wrench into patching the api traffic. I'm not sure how Reacts Hydration works, but It doesn't seem to be the case for the nofication count.

MV3 lacking support for direct body modification is indeed annoying, I didn't even know this was the case. Doing it via content script is still easy, however. Here's a minimal demo wrapped in a tampermonkey script (works only when ran before site JS runs, since it grabs window.fetch into a local)

Minimal Demo
// ==UserScript==
// @name         Tumblr Fetch Demo
// @version      2024-01-04
// @description  -
// @author       Temm
// @match        https://www.tumblr.com/*
// @icon         https://www.google.com/s2/favicons?sz=64&domain=tumblr.com
// @grant        unsafeWindow
// @run-at       document-start
// ==/UserScript==

console.log("Injected")
unsafeWindow.fetch = new Proxy(unsafeWindow.fetch, {
    apply: async (target, that, args) => {
        let res = await target.apply(that, args)
        let url = args[0].toString();

        if (url.startsWith("https://www.tumblr.com/api/v2/user/counts")) {
            console.log("Intercepting counts request!");
            let json = await res.json();
            json.unread = 0;
            res = new Response(JSON.stringify(json), res);
        }

        return res;
    }
})

The code in question that updates the title:

let z = _.redpopUnreadNotificationsOnTab ? u.inboxCount + u.notificationCount + u.unreadMessagesCount : u?.unseenPosts;
!this.state.isErrorPage && z && (L = `(${(0,
    k.Z)(z, 99)}) ${this.props.title}`,
    globalThis.navigator?.setAppBadge?.(u?.unseenPosts));

So it seems like wether the Inbox count is considered in the Window title is currently controlled via the redpopUnreadNotificationsOnTab feature flag.

leumasme avatar Jan 04 '24 18:01 leumasme

  1. We unfortunately can't use pageModifications to listen to changes to Tumblr's <title> element (which we want to do to keep our <title> up-to-date with the current page) because pageModifications only listens to <div id="root">

Scrapped code

import { keyToCss } from '../../util/css_map.js';
import { dom } from '../../util/dom.js';
import { buildStyle } from '../../util/interface.js';
import { translate } from '../../util/language_data.js';
import { pageModifications } from '../../util/mutations.js';

const followingHomeButton = `:is(li[title="${translate('Home')}"], button[aria-label="${translate('Home')}"], a[href="/dashboard/following"])`;

const customTitleElement = dom('title', { 'data-xkit': true });
const styleElement = buildStyle(`
${followingHomeButton} ${keyToCss('notificationBadge')} {
  display: none;
}
`);

const onTitleChanged = ([titleElement]) => {
  const rawTitle = titleElement.textContent;
  const newTitle = rawTitle.replace(/^\(\d{1,2}\) /, '');
  customTitleElement.textContent = newTitle;
};

export const main = async () => {
  pageModifications.register('head title:not([data-xkit])', onTitleChanged);
  document.head.prepend(customTitleElement);
  document.documentElement.append(styleElement);
};

export const clean = async () => {
  customTitleElement.remove();
  styleElement.remove();
};

I thought I remember instances in which the React code modified the title element rather than replacing it (and why wouldn't it, after all, if that's the minimal DOM diff). That being said, in a brief test this does seem to work with

  import { keyToCss } from './css_map.js';
  import { notificationSelector, postSelector } from './interface.js';
  const rootNode = document.getElementById('root');
+ const headNode = document.querySelector('head');

  const addedNodesPool = [];
  let repaintQueued = false;

  /* etc */

      if (!selector) return;

      if (modifierFunction.length === 0) {
+       const shouldRun =
+         rootNode.querySelector(selector) !== null || headNode.querySelector(selector) !== null;
        if (shouldRun) await modifierFunction();
        return;
      }

+     const matchingElements = [
+       ...rootNode.querySelectorAll(selector),
+       ...headNode.querySelectorAll(selector)
+     ];
      if (matchingElements.length !== 0) {
        await modifierFunction(matchingElements);
      }

  /* etc */

  });

  observer.observe(rootNode, { childList: true, subtree: true });
+ observer.observe(headNode, { childList: true, subtree: true });

marcustyphoon avatar Feb 02 '24 05:02 marcustyphoon

Working (I think) examples:

https://github.com/AprilSylph/XKit-Rewritten/compare/master...marcustyphoon:XKit-Rewritten:no-tab-numbers-pagemodifications https://github.com/AprilSylph/XKit-Rewritten/compare/master...marcustyphoon:XKit-Rewritten:no-tab-numbers-observer

marcustyphoon avatar Feb 03 '24 08:02 marcustyphoon

TBH I can't remember the rationale I had for only observing the React root in pageModifications in the first place. If we observe #root and head, why not just observe the whole document at that point? (We wouldn't even have to rename rootNode really, since we'd be going from observing #root to observing :root, lol.)

AprilSylph avatar Feb 04 '24 23:02 AprilSylph

I assume it was to avoid processing all of the script elements we're inserting with inject. I doubt that causes any problems, but it certainly did seem like a waste of cycles when every element React will ever insert will be inside the React root element!*

*or, you know, head, of course :D

marcustyphoon avatar Feb 05 '24 00:02 marcustyphoon

When you put it like that, observing just #root and head does seem more sane. Let's do it.

AprilSylph avatar Feb 05 '24 16:02 AprilSylph