Unified branches, extracted repeatedly used expressions.
Fortunately both branches had the same logic, except for placeholder position hint calculation.
* Fix placeholder position check
Because of the missed negation `placeholderPosition` could actually point to a different comment. As a result loaded comments would be displayed at a wrong position which could result in duplicated comments.
* Extract placeholder search code
Simple refactoring. The only notable change is that now technically `placeholderPosition` can be -1. In practice that should never happen, but I added checks anyways
* Separate network dependency injection module
- Moved network-related dependencies into a separate module
- Consolidated common dependencies to save resources constructing a http client/retrofit
* Separate construction of access token interceptor
* Create providers for Context and Application to be injectable
* Refactor AppModule and AppComponent
- Use component builder to store application context and provide to modules
- Optimise AppModule providers
* Use component factory to add component dependencies
* Updated network dependencies to singleton.
Add missing OAuth base url for oauth request
Co-authored-by: Kurian Vithayathil <no.reply@github.com>
Co-authored-by: Docile-Alligator <25734209+Docile-Alligator@users.noreply.github.com>
* Use and store sort type as enum for comments
* Use and store sort type as enum for posts
* Use sort type enum for search
* Remove unused reddit api methods
These were missed in the first "sort type" pr
* Remove unused isHiddenInRecyclerView Post field
Was used only for reads and was always false
* Remove unused isHiddenManuallyByUser Post field
Was only written to and read for serialization. Never used for any logic
* Show the current image index in gallery in PostFragment.
* Fixed issue where filters applied to same feed were behaving incorrectly (#1172)
* Fix ItemTouchHelper and gallery swipe gesture fighting each other.
* Show correct image in the gallery in ViewRedditGalleryActivity.
* Swipe to view images in a gallery in Card Layout 2.
* Fix gallery layout issues.
* Remove `commentData` argument that is always an empty ArrayList
* Return top level comments in addition to expanded comments after parsing
Since 0f1c4d loading more comments loads not only first level replies, but also deeper comments. Because of this `expandedComments` can contain those deep replies if `expandChildren` is true. Adding `expandedComments` to parent causes a bug because parent's children are supposed to be only next level replies. Because of previously mentioned changes that is not true.
Now expanding parent comment results in duplicate comments: one of them correctly comes from the parent of duplicated comment. The other one is shown because it is incorrectly stored in the parent of "load more comments" button.
This comment separates top level comments (fist level replies) and expanded comments. `expandedComments` are still used for display, but only first level replies are added to the parent
* Add debug assertion for children depth
Co-authored-by: Docile-Alligator <25734209+Docile-Alligator@users.noreply.github.com>
Co-authored-by: Aidan223 <110802888+Aidan223@users.noreply.github.com>
* Move mAccessToken and mAccountName initialization upper
* Marked INSTANCE as a volatile to be DCL compliant
* fix a typo in a method name and remove redundant switch
* remove redundant ifs
* removed DCL on RedditDataRoomDatabase as its controlled by dagger
* remove redundant ifs in PostGalleryActivity as nColumns can be either 2 or 3 only
* replace MediaStore.Video.Media.IS_PENDING with MediaStore.MediaColumns.IS_PENDING
* reverting simplified ifs as it going to be used in the future
* reverting HistoryPostViewModel
* Create and add to Retrofit SortTypeConverterFactory
* Clean up SortType class
Mark fields as final, annotate nullability
* Use SortType enums as query arguments
* Use optional query arguments to simplify code
Retrofit ignores null values, so there is no need to remove them from arguments manually
* Load more comments from /morechildren endpoint
Previous implementation requested comments from /api/info which returned the comments themselves but did not include any information about their children. Also /api/info does not allow to specify sort type. On the other hand /morechildren supports sort type and it will be added in a later commit.
I am not proud of this implementation, but I had to fight with both Reddit api response and existing code. The problem with api response is that it is a flat list of comments, not a tree structure. So the tree has to be rebuilt on our end. And the problem with existing code is that it merges "more children" node into its parent but then adds a placeholder anyways.
The code relies on the fact that parent comment will be located before any of its children in the response. The code sequentially processes comments, tries to find their parents and either adds them to the tree or puts in a "top-level" array which will be handled by outside code.
One possible problem is the removal of `depth` argument from parsing as I couldn't find a way to fit it in the new logic. However I also didn't experience any problems with it during my testing and the response seems to always contain depth key. Moreover current depth handling logic in ParseComment#parseCommentRecursion is broken because it does not increment depth when making a recursive call.
* Store moreChildren ids instead of fullnames
/morechildren endpoint accepts ids instead of fullnames so there is no point in converting ids to fullnames and back
* Send all comment ids to Reddit so it can select what to display itself
Sending all ids allows Reddit to sort them properly. Since the number of comments can be very bing, it requires using POST request instead of GET.
This commit changes the meaning of Comment#moreChildrenIds field, now it stores only ids of comments that are not yet loaded. This simplifies the code and removes the need for Comment#moreChildrenStartingIndex
* Fetch more comments with current sort type
It used position of viewholder during binding which could get outdated by the time user clicks on the button. This would result in retrieving wrong comment or even null. Replaced with getting comment based on current position of vieholder.
When extracting sort type loading logic the shared preferences that are used to load sort type got accidentally changed to the wrong ones. This resulted in always using the default value which is displayed as Best.
Fortunately the saving code was not changed so only reading has to be fixed.
Add a gradient from the primary theme color to transparent so that if a
subreddit or user profile has a very light background, the usually light
text and buttons are not obscured or in some cases invisible.
The gradients don't appear for immersive mode so that the app remains
immersive.
* Rename CONFIDENCE comments sort type to BEST and remove old BEST type
The Reddit API supports only CONFIDENCE sort type for comments but displays it
as BEST.
I renamed CONFIDENCE name to Best and added a migration step for loading
correct sort type.
* Clean up sortType usages in ViewPostDetailFragment
Removed unnecessary null checks, object creations and most case conversions
Condition in callback for loading avatar url is almost always true[1]. So it would load avatar even if the viewholder got bound to another comment.
Ideally the solution would be to update the comment just like now, then find current position of the comment and call onItemChanged. However you cannot call onItemChanged from onBindViewHolder and that is a problem because callback can be executed synchronously.
So instead we just check that viewholder is bound to some comment and that bound comment's author matches initially requested author.
[1] The only case I know when it is false is when that comment got deleted and its author got replaced with "[deleted]" before the callback got executed
* handle wiki links with dashes and index wiki page
* properly handle w vs wiki and links with wiki in it twice
* remove beginning and end of line tokens from wiki regex
* optimize wiki regex
Slidr works by adding its own view in the hierarchy and listening to touch
events in `onInterceptTouchEvent`. Once it detects movement in the correct
direction, it returns `true` and handles all the events itself.
Adding scrollable view detection to Slidr would solve the problem, but it is
not possible and would probably have performance impact.
Fortunately Slidr does not intercept the very first event, which is
ACTION_DOWN, and it reaches scrollable view. So the scrollable view itself can
decide if it should disallow the swipe.
This also has a performance benefit over `OnScrollChangedListener` because
the listener is triggered for every scroll of every view even if the child we
are interested in did not scroll. On the other hand `on(Intercept)TouchEvent`
is triggered only when the view is touched.
There is a possibility that swipe won't be unlocked if view never receives
ACTION_UP or ACTION_CANCEL. However the docs say nothing about the probability
of this happening. Anyways, one possible solution is to post a runnable that
will unlock swipe soon after locking.
Child comment expanding was broken because it did not take into account children
of children of children and deeper levels of comments when calculating new
comment's position.
Replaced with a simple tree to list conversion in pre-order.
* Persist read permission
ACTION_OPEN_DOCUMENT_TREE grants both read and write permissions, but they are
granted only until device reboot unless app persists them.
Once read permission is lost, app cannot check if folders exist in DownloadMediaService.
But it can still create new folders because it has write permission. This results
in duplicate folders.
* Remove unnecessary FLAG_GRANT_WRITE_URI_PERMISSION
This flag is ignored when used with ACTION_OPEN_DOCUMENT_TREE
Implementation is inspired by already existing in Markwon image and link processing
but has to work around some limitations of writing an external plugin.
The first one is storing brackets ourselves. Stored brackets need to be cleared
when a new block starts. Markwon does it in MarkwonInlineProcessor but there is
no callback that we could use. Clearing storage from our own block parser is
unreliable as it is not guaranteed to be called. Instead, every time we need to
access the storage we compare current block with the last used block and clear
storage if necessary.
The second problem is actually a feature of Markwon that it applies spans in
reverse order of calls to MarkwonVisitor#setSpansForNode. This causes other spans
like links and code to be drawn over spoilers making them visible. Adding spans
with a different priority doesn't help as it would require negative priority.
Instead we just remove all the SpoilerSpans from the final string and add them
again, so they are applied last as we want.
* Add todos to places that need more markdown fixes
* Parse spoilers and headings in sidebar
* Assign anonymous MarkwonPlugin to a variable
Prepare code for a future refactoring
* Assign click listener lambda to a variable
Prepare code for a future refactoring
* Add function for creating Markwon with full markdown
All the builders had the same plugins applied to them, except for
BetterLinkMovement. But it is safe to add the plugin as it just adjusts
link interactions.
Also some plugins are now applied in a different order but it doesn't
change anything in this case.
* Add function for creating Markwon with only links support
* Extract UrlMenuBottomSheetFragment creation
* Add functions for creating MarkwonAdapters
* Replace linkify with newInstance for BetterLinkMovementMethod
Because varargs weren't used, the two methods are identical
When the class name is relative, Android tries to resolve it against applicationId.
However it does not match the package because of `.debug` suffix so it tries
to load the wrong class. This results in ClassNotFoundException and a crash.
Using fully qualified class name fixes it as the system can use the class name
as is.
* Prioritize clicks on hidden spoilers over links
Extend BetterLinkMovementMethod to override selection of span that will be
touched and give spoilers and links following priorities:
1. Hidden spoiler
2. Non-spoiler span (i.e. link)
3. Shown spoiler
#609
* Ignore long clicks on spoilers
Ignore long clicks if it is a SpoilerSpan. Also don't apply highlight
because it breaks spoiler effect.
#529
* Rewrite spoiler parsing to properly support nested spoilers and code blocks
Parse all the spoilers, ignoring spoiler brackets that intersect with code
spans. Detect all the spoilers that are nested and mark them accordingly.
Delete all spoiler brackets that were matched. Add SpoilerSpans for non-nested
ranges.
* Simplify offset calculation
* Display comment body the same way as post body when replying
There are two views that support markdown, one of them was used to display post
titles and comments, the other - post bodies. The views are configured differently
even though post and comment bodies should be displayed the same way. Now post
and comment bodies are displayed by the same view.
* Rename extra keys from TEXT to TITLE
Now these extra keys are used only by post title, reflect this in the name.
* Remove markdown support from post title view
* Fix reply styling
Co-authored-by: Docile-Alligator <25734209+Docile-Alligator@users.noreply.github.com>
* Allow parallel installation of debug and release versions
...by adding '.debug' suffix to application id
* add separate app name resource for debug build
Leak found using LeakCanary. Steps:
1. Enable the LeakCanary dependency.
1. Open the app.
1. Go to the "All" tab.
1. Open any post, and go back to the post list.
Leak trace:
```
2022-09-04 17:56:05.904 32018-32018/ml.docilealligator.infinityforreddit.debug D/LeakCanary:
┬───
│ GC Root: System class
│
├─ android.net.ConnectivityManager class
│ Leaking: NO (a class is never leaking)
│ ↓ static ConnectivityManager.sInstance
│ ~~~~~~~~~
├─ android.net.ConnectivityManager instance
│ Leaking: UNKNOWN
│ Retaining 114 B in 5 objects
│ mContext instance of ml.docilealligator.infinityforreddit.activities.ViewPostDetailActivity with mDestroyed = true
│ ↓ ConnectivityManager.mContext
│ ~~~~~~~~
╰→ ml.docilealligator.infinityforreddit.activities.ViewPostDetailActivity instance
Leaking: YES (ObjectWatcher was watching this because ml.docilealligator.infinityforreddit.activities.
ViewPostDetailActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
Retaining 1.8 MB in 27752 objects
key = 22e99901-9689-4f70-b88c-092a4a7efad9
watchDurationMillis = 5518
retainedDurationMillis = 517
mApplication instance of ml.docilealligator.infinityforreddit.Infinity
mBase instance of androidx.appcompat.view.ContextThemeWrapper
```
Solution based on [this StackOverflow answer](https://stackoverflow.com/a/41431693)
* Update multireddit apply filter text
A "/" is needed at the end of the path in order for the filter to be applied correctly to the multireddit. Also, any characters that are not lowercase will also make it not apply properly.
Leak found using LeakCanary. Steps:
1. Enable the LeakCanary dependency.
1. Open the app.
1. Open any post image, and go back.
According to `Piasy/BigImageViewer` documentation,
[Initialize section](9cc045e814/README.md (initialize)),
the app context must be used to avoid memory leaks.
Leak trace:
```
2022-09-04 19:51:38.154 13332-13332/ml.docilealligator.infinityforreddit.debug D/LeakCanary:
┬───
│ GC Root: Thread object
│
├─ android.os.HandlerThread instance
│ Leaking: NO (PathClassLoader↓ is not leaking)
│ Thread name: 'LeakCanary-Heap-Dump'
│ ↓ Thread.contextClassLoader
├─ dalvik.system.PathClassLoader instance
│ Leaking: NO (BigImageViewer↓ is not leaking and A ClassLoader is never leaking)
│ ↓ ClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│ Leaking: NO (BigImageViewer↓ is not leaking)
│ ↓ Object[257]
├─ com.github.piasy.biv.BigImageViewer class
│ Leaking: NO (a class is never leaking)
│ ↓ static BigImageViewer.sInstance
│ ~~~~~~~~~
├─ com.github.piasy.biv.BigImageViewer instance
│ Leaking: UNKNOWN
│ Retaining 969.9 kB in 14812 objects
│ ↓ BigImageViewer.mImageLoader
│ ~~~~~~~~~~~~
├─ com.github.piasy.biv.loader.glide.GlideImageLoader instance
│ Leaking: UNKNOWN
│ Retaining 969.9 kB in 14811 objects
│ ↓ GlideImageLoader.mRequestManager
│ ~~~~~~~~~~~~~~~
├─ com.bumptech.glide.RequestManager instance
│ Leaking: UNKNOWN
│ Retaining 969.9 kB in 14808 objects
│ context instance of ml.docilealligator.infinityforreddit.activities.ViewPostDetailActivity with mDestroyed = true
│ ↓ RequestManager.context
│ ~~~~~~~
╰→ ml.docilealligator.infinityforreddit.activities.ViewPostDetailActivity instance
Leaking: YES (ObjectWatcher was watching this because ml.docilealligator.infinityforreddit.activities.
ViewPostDetailActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
Retaining 966.2 kB in 14703 objects
key = f69c74cc-521e-4f6c-b5c8-8f787e27df75
watchDurationMillis = 5547
retainedDurationMillis = 541
mApplication instance of ml.docilealligator.infinityforreddit.Infinity
mBase instance of androidx.appcompat.view.ContextThemeWrapper
```
* Copy heading parser and adjust it to match Reddit behavior
Unlike CommonMark, Reddit does not require space after #. This behavior is
coded in a private static function, so the only way to override it is to
copy everything and use the modified copy instead of the default parser.
* Use RedditHeadingPlugin instead of regexes
* Apply plugins to post body when writing a comment
This fixes display when writing comment to a post
that contains spoilers or headings without space
* Apply plugins to parent comment body when writing a comment
This fixes display when replying to a comment that contains strikethrough text
* In anonymous mode, fixed bug in CheckIsFollowingUser.java that does not check if accountName is null or not, which prevents room from retrieving SubscribedUserData to check if the user is followed or not.
Also updated Room version to 2.4.2 to be compatible with M1 macbook.
* Simplified SubscribedUserData accountName null check code
