search

itkach / aard2-android

Aard2 for Android, a simple dictionary app
GNU General Public License v3.0
464 stars 98 forks source link

Added link sharing functionality (#103) #127

Closed tswistak closed 2 years ago

tswistak commented 3 years ago

Hi,

I've decided to add "Share link" functionality, raised in issue #103. I know there is already a possibility to share any link from the entry by long-pressing it. Still, this functionality is not very obvious (I've learned about it from the code even though I was using the app for some time), and I thought a separate "Share" button could be useful.

The implementation is based on downloading the dictionary entry in the background and searching for an anchor tag with id="view-online-link". All dictionaries made out of MediaWiki have an original link with such id. From what I've seen in the dictionaries list, most of them are from MediaWiki pages, and those that aren't, don't contain any other reference to the original. Of course, if, with some time, slob files would keep a reference to the original resource, it can be rewritten, but from what I've found out while using the Python's slob tool, dictionaries don't keep such metadata for entries.

I hope you like it, and if you have any comments on the code, I can fix it (I haven't programmed in Java for years, so I could do something wrong).

itkach commented 3 years ago

I agree that long-press to share a link is difficult to discover and most people probably don't know about it. But clicking on a link (external ones are clearly marked same way as in Wikipedia) and sharing from a browser is fairly straightforward. So this feature doesn't seem to be a must-have.

The proposed implementation is problematic, mainly because parsing html with regular expressions is problematic in general, even if you're not looking to parse html fully. The regex is likely incorrect and doesn't work in all cases and it is likely to cause unexpected performance issues. Other issues include launching multiple instances of an expensive background task in a callback that may be called frequently (depending on user activity), reloading potentially large content, recompling regex on every call, assuming content is utf8 (not necessarily), assuming it is html (not necessarily), using printStackTrace() while the rest of the code base does not...

A better approach would be to query the document loaded in WebView instance using querySelector() from injected javasript that would call back into Java code. Another approach could be constructing external URL using dictionary's source tag and title. The first one is fairly complex, and the second one is not guaranteed to produce correct link though. Not worth the effort, IMO.

tswistak commented 3 years ago

Fine, I understand. If it's not needed feel free to close the PR, I won't be mad about it, it still was a nice challenge to do this feature 😄 . About the solution, I was thinking also about the second approach with JS, but unfortunately evaluateJavascript needs SDK 19 (project's minsdk is 15). That's why I decided to go with fetching HTML and Regex, since there is no problem with using them on older Androids.