Skip to content

fix(webview): rescroll target post once content swap settles (#631) #810

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.

Sign up for GitHub

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

Open
jim-daf wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(webview): rescroll target post once content swap settles (#631) #810

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions Awful.apk/src/main/java/com/ferg/awfulapp/webview/AwfulWebView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,45 @@ public void runJavascript(@NonNull String javascript) {
}


/**
* Issue #631: the thread page is rendered by swapping innerHTML, so
* neither WebViewClient.onPageFinished nor any other lifecycle
* callback fires when the actual posts arrive. This helper installs
* a MutationObserver that watches the post container and, once it
* has been quiet for a beat, scrolls the target anchor back into
* view. Call this after {@link #setBodyHtml(String)} when the
* caller knows which post the user came in on.
*
* @param anchorId the DOM id of the target post (no leading hash).
* Pass null to fall back to the URL fragment.
*/
public void scrollToTargetAfterContentSettles(@Nullable String anchorId) {
String target = anchorId == null ? "null" : "'" + anchorId.replace("'", "") + "'";
runJavascript(
Comment on lines +174 to +176
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper is not referenced anywhere in the app yet (no call sites found), so the behavior described in the PR title/"Closes #631" won’t actually take effect. Either wire it up from the intended caller (e.g., ThreadDisplayFragment after setBodyHtml) or adjust the PR title/description and avoid closing the issue until it’s integrated.

Copilot uses AI. Check for mistakes.
"(function(){" +
"var anchorId = " + target + ";" +
Comment on lines +175 to +178
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anchorId is interpolated into a JS string literal by stripping only single quotes. This can still break the injected JS (e.g., newlines/backslashes) and also changes legitimate IDs containing '. Prefer proper JS string quoting/escaping (e.g., via JSONObject.quote(anchorId) or a dedicated escaping helper) instead of removing characters.

Copilot uses AI. Check for mistakes.
"var quiet;" +
"function jump() {" +
"var id = anchorId || (location.hash || '').replace('#','');" +
"if (!id) { return; }" +
"var el = document.getElementById(id);" +
"if (el) { el.scrollIntoView({block: 'start'}); }" +
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

el.scrollIntoView({block: 'start'}) is not supported on older Android WebView/Chromium versions (the project already mentions API 21 / Chromium 37 compatibility in thread.js). This call can throw and prevent the rescroll entirely. Use the legacy boolean form (scrollIntoView(true)/scrollIntoView()) or feature-detect the options-object overload and fall back accordingly.

Suggested change
"if (el) { el.scrollIntoView({block: 'start'}); }" +
"if (el) {" +
"try { el.scrollIntoView({block: 'start'}); }" +
"catch (e) { el.scrollIntoView(true); }" +
"}" +

Copilot uses AI. Check for mistakes.
"}" +
"var root = document.getElementById('container') || document.body;" +
"var observer = new MutationObserver(function(){" +
"clearTimeout(quiet);" +
"quiet = setTimeout(function(){" +
"observer.disconnect();" +
"jump();" +
"}, 150);" +
"});" +
"observer.observe(root, {childList: true, subtree: true});" +
// Fallback: if no mutations fire within 2s, jump anyway.
"setTimeout(function(){ observer.disconnect(); jump(); }, 2000);" +
Comment on lines +195 to +196
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2s fallback setTimeout will still fire even if the observer already detected "quiet" and called jump(), causing a second jump later (potentially yanking the user back after they started scrolling). Track the fallback timeout ID and cancel it when jump() runs, or gate jump() with a didJump flag.

Copilot uses AI. Check for mistakes.
"})();");
}


/**
* Calls the javascript function that displays the current body HTML
* <p>
Expand Down
Loading