ka-extension / ka-extension-ts

A browser extension for Khan Academy.
MIT License
19 stars 12 forks source link

KA updated class naming format, extension's broken #175

Closed MatthiasPortzel closed 5 years ago

MatthiasPortzel commented 5 years ago

You know the drill. No new features, just all class names are different so the extension broke again. A lot of querySelectorPromises calls need to be updated, as well as a lot of CSS.

lukekrikorian commented 5 years ago

I've been thinking about this problem for a while now, and I think the solution would be some way of reversing the hashing that aphrodite performs on class names. Of course, the only solution that I can currently think of is some hacky document.querySelector code, but I think it would be a lot better than our current solution.

MatthiasPortzel commented 5 years ago

Honestly, that would be really cool. I don't think it's feasible, because the class name hash is effected by all rules that apply to the class. Some of my interesting testing, on Runkit. Unless I don't understand what you're proposing.

JettBurns14 commented 5 years ago

Maybe instead of selecting each button/element by class, we try to select the parent containers of all these elements and delete their children on load, and replace them with our own HTML buttons that do the same things. We could potentially apply this to many of the elements we modify, and it theoretically should give us more control over them. Thoughts?

lukekrikorian commented 5 years ago

That would probably interfere with their React setup more than we'd like (if I'm understanding you)

lukekrikorian commented 5 years ago

@MatthiasSaihttam, it looks like KA updated Aphrodite to a recent version which removes the original classname prefix e.g button_jiosjd -> jiosjd, so my previous idea wouldn't work any longer

lukekrikorian commented 5 years ago

Here's a quick (currently very hacky) idea I came up with that I think has some potential for expansion: https://ourjseditor.com/program/7XRsMV

MatthiasPortzel commented 5 years ago

The thing is, as long as we refer to things by class name, it's going to need to be updated when class names change. I don't see a significant advantage to using something like that PoC, Luke, over just using query selectors.

It would be cool if we went away from class names and found things by content. (Like find elements with the text "Vote Up" to find the vote button.) My primary concern with that is that it wouldn't work for other languages, which I think is a dealbreaker.

I'm currently thing the best option is going to be a script of some sort that one of us would run to find the new class names or identifiers for the elements we need, and then update a list that gets built into the source. So we'd still have to push an update, and we'd still get elements by KA's static class names, but it would speed up the update process. Right clicking an element in the inspector lets you copy its CSS path, giving us a query selector for elements less manually.

lukekrikorian commented 5 years ago

The idea with my PoC is to be easier to parse over extremely complex query selector strings, to clarify. The idea of manually updating some external list of elements doesn't exactly appeal to me. We'd need to pull in a headless library that can parse JS like puppeteer to execute KA's frontend code and we'd still need query selectors to find the element's whose classnames we need to reference. Unless the idea is that we'd have to manually update the list ourselves, which doesn't sound much better imho

lukekrikorian commented 5 years ago

@MatthiasSaihttam any update on your branch?

MatthiasPortzel commented 5 years ago

I figured out that that fancy css selector I wrote isn't going to work :/