Force news hubs links to open in default OS browser #3276
Conversation
| "document.onclick = function (elt) {" + | ||
| " document.querySelectorAll(\"a[target]\").forEach(e => {" + | ||
| " if (!e.href.includes(\"javascript\")) " + | ||
| " e.href = \"javascript:java.openUrl('\" + e.href + \"')\"" + | ||
| " });" + | ||
| "}"); |
There was a problem hiding this comment.
I don't quite understand this function. So everytime the document is clicked it is swapping out the href of all the links in the document right? When someone first clicks a link wouldn't this not work because the onclick event propagates from the inside out? Also this assumes the event did not consume the event and stop propagation right?
There was a problem hiding this comment.
I wonder if MutationObserver is supported by webview and we would rather use that. https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver
There was a problem hiding this comment.
Yea your understanding is correct, its not great. it does seem to work on all the links in the news page, even on first click. Yes it assumes the event was not consumed.
Im happy to explore if mutationobserver will work because I dont like this solution at all, it was a starting point. Another option I explored is finding the href of the clicked element which was infeasible since sometimes its the clicked element, or a direct parent or child, or a descendant of either.
|
@Sheikah45 Mutation observer works nicely to fix any links added to the page Unsure but perhaps there is still a concern with a race condition between the java code and the js hook? Where a link can get added after the java code runs but before mutation observer is added. |
…nto bugfix/#3262_fix_links
The race condition concern would be fixed by first adding the mutation observer and then going through and changing all the links already added to the page. This would guarantee that all the links are set since we observe and then change the preexisting ones. |
| for (const node of addedNodes) { | ||
| if (node?.querySelectorAll) { | ||
| links.push(...Array.from(document.querySelectorAll("a"))); | ||
| } | ||
| } |
There was a problem hiding this comment.
Only thing besides switching this and the href setting above. But don't we only need to push node.querySelectorAll("a")?
There was a problem hiding this comment.
I dont think so
It returns a nodelist and pushing that alone didnt seem to explode it correctly
There was a problem hiding this comment.
node of node list is an array? But you are already running queryselector on it
|
We could also prevent the race condition by writing the logic to handle pre-existing links in the js script |
Fix the underlying issue preventing links opening correctly in #3262
This PR doesn't add a setting to control it, I assume everyone want the links to open in the OS browser.