Skip to content
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.

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

Incognito for chromium browsers #301

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

PabloOQ
Copy link
Collaborator

@PabloOQ PabloOQ commented Dec 17, 2023

#115

This is still a WIP, there are a lot of things to be done yet, but I don't want to keep these and not upload it (again). I think it would be best to check this code first, make the necessary changes and then build on top of it, that's why there are some TODOs in the code. The strings from the OpenModule config have not been updated yet because of this.

Added support for all firefox(fenix) forks.

Added support for chromium forks, it launches the incognito activity and then stores the URL in the clipboard, the user is expected to paste it in the search bar, we then restore the clipboard contents.

No screenshots, nothing in the GUI has changed (yet).

Here is a diagram that kind of explains the code, there are some differences because threading is not linear.

flowchart TD
    incognito(User opens URL in incognito mode)
    store[Store previous clipboard content]
    load[Load incognito URL into clipboard]
    wait[Wait]
    interrupt{"User requested
    to open another
    incognito?"}
    check{"Is the clipboard
    content the same
    as the loaded URL?"}
    restore[Restore stored clipboard content]
    replace[Store to later restore]
    finish(Process finished)

    incognito --> store --> load --> wait
    wait --> check
    %% User does nothing
    check --> |Yes| interrupt
    %% Uses the clipboard
    check --> |No| replace
    replace --> interrupt
    interrupt --> |Yes| load
    interrupt --> |No| restore

    restore --> finish
Loading

Can't believe GitHub has support for these, I was going to link to the official mermaid web.

The order I would advise checking the files (as is the one in which I made them) is:

  1. ClipboardBorrower
  2. AutoClipboard
  3. UrlHelper
  4. Incognito

Clipboard Borrower is meant to handle the logic of borrowing the clipboard, the user should be able to freely use their clipboard even when we are borrowing it, so we keep track of the last item the clipboard had that we did not put in there. At the very end we restore it. The only situation a user would not have access to their clipboard content is inmediatly after opening an URL in incognito, they would have to wait until we restore the clipboard. This could be solved in the future by using the SemiAuto form UrlHelper, which would restore it after X seconds but if you tap the bubble it would restore it immediately.

Auto Clipboard borrows the clipboard inmediatly and restores it after X seconds, if called again before this, it cancels the thread and starts the timer again.

UrlHelper keeps track of the options available for this "compatibility" we made:

  • AutoClipboard: Restores the clipboard after a set amount of time.
  • ManualClipboard: Restores it when the user taps in the bubble (not implemented).
  • SemiAutoClipboard: Restores the clipboard after a set amount of time, if the user taps the bubble earlier, it is restored earlier.

Some things to be considered:

  • Auto should never use a bubble, it doesn't make much sense to not use instead SemiAuto.
  • Auto should only use: nothing (current implementation, only for android 9 or less), notifications (android 10 or more), acessibility service (don't know yet which versions)
  • Manual and SemiAuto should not use notifications, it's just a total hassle to swipe down and the find the notification to tap it, a bubble is just more convenient.

Incognito now has a static section in which scripts can be added to detect apps that can run incognito and modify the intent to do so, I thought this could be extracted into a database file, but I didn't do it because it's probably mesier and, honestly, aside from fenix and chromium, we don't know how other apps work with incognito, it could be more complex.

As a side note, this whole system can help if an application can't open the URLs it is supposed to and has access to a search bar or something similar, which is unlikely, but the option is there.

I think that's all, after you've made all the necessary changes, just ping me and I'll come back to do what's left (bubble and notification), after that I'll push and then I might look at the accesibily service, but I don't promise anything. Take all the time you need.

Also as stated in this comment there is some kind of compatibility with customs tabs, but I have not made any tests with it yet, as I want everything else working correctly first.

Added fenix forks support
Added chromium support
@github-actions github-actions bot added Core Change the core code Assets Changes some assets labels Dec 17, 2023
Copy link
Contributor

This PR builds correctly, here is the generated apk.
This unsigned version can be installed alongside the original app and should only be used for testing the changes, not for daily usage.

Download testing apk

You must be logged in for the link to work.
The link will expire at 2023-12-31T15:48:40Z.


This is an automatic comment created by a Github Action

@TrianguloY
Copy link
Owner

Note: I haven't forgot about this. But due to the complexity and their experimental value I'm going to prioritize other tasks over this, sorry.
I may add it as an experimental feature though.

In any case, don't hesitate to keep working on it and ask any question you may need (I need my computer to fully review a pr, but I read all messages from my phone so it is much much faster)

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Dec 31, 2023

I'll continue with the bubble then, it should bring support to all android versions.

Copy link
Contributor

github-actions bot commented Feb 28, 2024

This PR builds correctly, here is the generated apk.
This unsigned version can be installed alongside the original app and should only be used for testing the changes, not for daily usage.

Download testing apk

You must be logged in for the link to work.
The link will expire in 14 days, at Sun Nov 3 15:05:59 UTC 2024.


This is an automatic comment created by a Github Action

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Feb 28, 2024

Finally, I added support for borrowing a clipboard using a bubble. I started working on this like a month ago and it took 2 weeks, since then I couldn't find the time to clean the code. I think that should be it for some time. There is still work to do but, if anyone wants to test this they should be able to, they need to first give permission manually to the app so it can draw over other apps, or it will crash. Currently there is no way to change the helper used, this one creates a bubble that when tapped will restore the contents of the clipboard, it will also do the same after 10 seconds of being created.

So, things that still need tweaking:

  • There are some strings here and there that are hardcoded, they are properly tagged with either FIXME or TODO.
  • To check if we have access to the clipboard I access directly to it, which triggers the toasts, this and the need to keep previous contents of the clipboard stored make it so that for every release, borrow or canUseClip call it shows 2 toast each, for a total of 6 every time the user wants to open chromium in incognito.
  • I don't like at all the way I made to choose the helper getHelper, it is really bad. But I did not want to keep pushing this any longer.

Now, the "janky" parts. Because we are trying to use the clipboard when we shouldn't, I had to make some weird workarounds. If we are not focused we create a Bubble so we get focus, and before it shows we do what we need with the clipboard and we proceed to kill the bubble, that means that we need to wait for the UI thread to start creating the Bubble to complete background operations on the clipboard. We also can't reuse the bubble that is visible because of its flags, I spent a lot of time trying to figure it out, but I think is not possible without ruining the user experience, if we have focus on the bubble, we can't use the keyboard on other apps.

I also don't know if using a notification will help avoid using a fake bubble, but it is very unlikely.

I changed the logic to check if the clipboard has changed, in debug it doesn't check the label, in release it is just equals. This is because the emulator keeps syncing the clipboard of the PC and Android.

Things that still need to be done:

  • Remember the position of the bubble.
  • Make it impossible so the user can move the coordinates of the bubble outside the screen (although the view doesn't go out).

I think that's all.

@github-actions github-actions bot added the Translation Changes for a non-english locale label May 29, 2024
@PabloOQ
Copy link
Collaborator Author

PabloOQ commented May 29, 2024

So, this should be almost everything done. The only thing that I haven't added is keeping the coordinates of the bubble, which last time I tried I couldn't do properly (also tried to keep in mind foldables).

Added a button in the open module config to change the settings, and also, there is an explanation, however... is a huge wall of text, don't really know if it can be summarized as the feature is really complex, and I feel a lot of that information can be relevant to the end user.

Because of that complexity I think an option (worst case scenario) could be to just hide this feature, I still use the prototype frequently, and while I can't say I find it convenient is not as inconvenient as this hacky code could make it seem, and someone could put this feature to use.

Also there is now a "None" helper, when selected if the user tries to open the URL in chrome in incognito it will work just like it currently does, it opens it without incognito.

Let me know what you think when you can (don't worry, there is no hurry).

@PabloOQ PabloOQ removed the Translation Changes for a non-english locale label Jun 1, 2024
@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Jun 3, 2024

While I was at it I also made a working version that uses the accessibility service, so it is fully automatic, in the emulator it was really sloppy, but on my phone it was great.
The code is not store specific (yet), and I won't upload it until you tell me, this pr, another pr, or just not, as I remember you saying that you weren't sure about using the accessibility service.
The feature is in this branch . Feel free to check it out.

@TrianguloY
Copy link
Owner

All the work here is incredible, and being able to make it work was no easy task I'm sure.

I've think about it, and how to include it in the app without permission issues or other things that play store may not like. And I had an idea that, now that the feature is complete, I think makes even more sense. Tell me your opinion.

The idea is: create a separate app.

This app would have a main screen, that will explain the purpose and the usage (that wall of text you mention for example) and maybe a button to try and configure.

Then it would have a transparent single activity that will perform the "open in a private session in chrome" feature from the provided link. From the user perspective they could either "send" the link to the activity (and it will be opened) or click and choose (if they don't have a default browser). Similar to other apps that also allows you to "open a link" differently.

The separate app would also allow it to have as many permissions as needed, or to create different versions depending on the store. Plus maybe adding support for other browsers!

But the main benefit is that, by being separate, you could even use it with other "url managers", or even none (with the share option).
Even in that case, however, URLCheck can have special code to detect this app and "integrate" with it (by for example removing from the list of available apps and calling it when using the incognito button with chrome).

Sorry if this seems more like a "I don't want this on the app" response. It's not. It's just that this functionality is way more complex than a simple "button", and I'm afraid that adding it to the app itself will make it very difficult to maintain. Plus I genuinely think that people will benefit from this feature independently.

If you like the idea, I can help you setup an independent app with the code. And feel free to reuse any util/class from URLCheck of course (like the readme says: as long as you mention me somewhere it's enough :)

Plus I can also try to publish it on my Play Store account if you don't plan to create one yourself (although in this case I recommend to start with FDroid first).

Let me know what you think. If you like the idea of a separate app or not. In either case I'll support you 👍

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Jun 4, 2024

I understand, to be honest I was feeling the same while coding this, I could only think about how all this originates from a single boolean, which is there on purpose for some reason, and how it made these unnecesary permissions necessary, and while I tried my best to make good code, the "nature of fighting back" against something not meant to be that way made it impossible to have code as clean as I had hoped.

It's a shame, making this a library won't fix the problem either, so I guess the only alternative is making it a separate app, as you propose. This is one of the most requested features, and it is really useful, so I think it should be released, if that means a new app I'm okay with it. Although it will take some time and I don't even know where to start.

I would like, if possible, to at least have an experimental apk of this branch, or another one more up to date, on the releases tab of the github. This way if anyone wants to test it, there is the possibility of having early feedback. We can store the branch on the shelf too.

@TrianguloY
Copy link
Owner

TrianguloY commented Jun 4, 2024

Ok! No problem. Let's do the following then:

  1. Update the branch with the latest master changes (to be precise merge master into incognito-clipboard). This will make it work with the latest version, and help with future changes. I'm experienced with merges, and I should probably be able to do it myself without issue this weekend, but I'll wait for your comment (just in case your already started it).
  2. Split the code as much as possible. Try to separate everything into an independent package, and try to have the minimum "modifications" to the standard code. Basically try to reduced the number of modified files in the pr as much as possible. This may be hard in some cases, but from what I see it is already highly separated.
  3. Maybe move the code to a separate version. Versions allows apps to have a common core but different "versions" which can include new classes, alternate assets, or even manifest modifications. This would be perfect and even though I've only used versions for minor changes I know more or less what to look for.
  4. And finally, merge the PR.

This should allow to keep the original app and additionally build the modified version without issue (and maybe even publish it as apk free on Github if I finally have the time to implement the apk releases...ahem).

Anyway, let's start from the beginning: do you want me to do the branches update or you want to try it yourself?

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Jun 5, 2024

do you want me to do the branches update or you want to try it yourself?

I'll do the merge myself, I want to add the accessibility service too, and I have some fixes in that branch, then I'll see how to split it properly, I think the incognito class could be a problem as I added a lot of the code there.

@github-actions github-actions bot added Translation Changes for a non-english locale Github Changes to github thingies Gradle wrapper Change on Gradle wrapper folder Store file Changes store assets Automation labels Jun 6, 2024
@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Jun 10, 2024

I should lock master (maybe it is already? I don't remember)

I think it is, tried to fix one hardcoded string a month or so ago and couldn't (don't worry, you fixed it at some point later)

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Jun 17, 2024

So, I moved everything to a new package, tried to make it like a library, that's why there are some repeated folders. Things that are still outside said package are resources, everything else is nicely packed together.
Now I should make it a real library, and move the resources there, also I would need to duplicate some code like the enum interface and methods as well as GenericPrefs.

Maybe move the code to a separate version

I understand this is a specific tool, but can't find out how to start on this since the word is so common, my searches yield no result. Could you point me on how to do this?

Also I found a bug with the clipboard borrower on my personal phone, but for reasons I will not explain I can't debug it right now, it will have to wait (just writing it somewhere to not forget about it).

@TrianguloY
Copy link
Owner

Maybe move the code to a separate version

I understand this is a specific tool, but can't find out how to start on this since the word is so common, my searches yield no result. Could you point me on how to do this?

Oh, search for "gradle flavours", and flavourDimension (the code I was using as reference uses "version" as one of those flavours, but maybe any word can be used). It's apparently similar to buildTypes, but has some different considerations. Honestly I don't know much about them, but I know one of those allows to add extra modules...somehow

@TrianguloY
Copy link
Owner

Is this still in progress or is it now ready to be reviewed?
I'm asking because if this is still in progress I think it is a good idea to mark the pr as a draft (don't know why I haven't suggested that before...)

I also see some conflicts, but I can try to fix them myself if needed.

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Aug 29, 2024

Is this still in progress or is it now ready to be reviewed?

It is still WIP, I haven't been at home for a few weeks (in fact I got the mail notification just after coming back), but before that, I tried at least 3 times to move the code to a separate "Android Library" so then it can be easily transfered to the other app. However, I can't make it work, since I have never made an app from scratch there are some things I couldn't figure out (like making the resources of the library work properly, and then actually calling the library from the main app). I was going to give it one last shot before asking for help.

mark the pr as a draft

Sure, I'll do it myself while I'm here.

I also see some conflicts, but I can try to fix them myself if needed.

Do not worry about the conflicts, those I will fix myself when everything is ready to be merged.

@PabloOQ PabloOQ marked this pull request as draft August 29, 2024 19:04
@PabloOQ PabloOQ changed the title Added auto clipoard borrower to incognito Added auto clipboard borrower to incognito Aug 29, 2024
@PabloOQ PabloOQ changed the title Added auto clipboard borrower to incognito Incognito for chromium browsers Aug 29, 2024
@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Sep 5, 2024

So, this time I had more luck and found what I needed, so I did it myself!

Anyways, before the module/library thing, there are 2 bugs currently I discovered yesterday, both I cannot debug:

  • Sometimes, the accessibility service doesn't start or something similar. This happened after rebooting my device. Since there is no way for us to control the life cycle of the service, that task lies on the OS, we can't start it, so the only fix is to reboot the device, I even tried to enable and disable the service multiple times. It is not easily reproducible because it happens at random, and it is out of our control, so I think this can't be solved.
  • If using the clipboard as helper, if it is empty (easily doable with Simple Clipboard Editor), it thinks we can't access it (I coded it that way, whoops). I found the bug on my phone, which for reasons, I can't connect to my PC and the emulator keeps the clipboard with a label (even when I told it not to sync with my PC), so it is always non-empty. Therefore I have no way to debug this properly, I will still try to fix it, but I can't test if it works.

Now, the module, this branch starting 3rd of September, contains alll the small changes made to create the module library. Some commits are worth checking, as I'm not sure I did it right:

  • Fix incorrect library naming, copy missing classes and functions: There are some classes copied from URLChecker, as I was using them previously, the module is supposed to work without URLChecker so the code must be either packaged with the module or on another module that substitutes the duplicate code. Duplicate code is preceded by a comment:
// ---
// Everything, starting here, is copied from URLChecker
// TODO: move to external library?
// ---
<!-- -->
<!-- This resource is copied from URLChecker -->
<!-- TODO: move to external library? -->
<!-- -->
  • Extract calls to ForceURL and Add assertions to avoid calling methods when IS_INCOGNITO is false (whoops, I forgot to merge those): The IncognitoDimension.java file, contains minimal code, the purpose of the file is to wrap all import ...forceurl here, so only calls to the module are allowed here. There are 2 versions, gradle chooses which file will use based on the flavor. The methods of the file should only be called when IS_INCOGNITO is true, that way the code is not scarced without reason.
  • The rest of build.gradle I copied from URLChecker.
  • The old naming scheme for the strings doesn't make much sense in the new module.

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Oct 4, 2024

I am almost finished with this.

First, a feature I wanted to add but is missing. For foldables, I was going to store the position of the bubble depending on which screen it is on, but it needs androidx and even then it wasn't complete, to not drag this any longer, I'll investigate this in the future.

Then, in local I have already merged with master, but I had one issue that caught my attention, and I need help deciding which is the right way to solve it. So, in this commit (the URL puts you on the exact line, no need to search for it), the intent the app would send doesn't intent.setPackage(chosen) anymore and instead relies on intent.setComponent(chosen.getComponent). This is already fixed in my local branch, but, the incognito feature is separate from the app itself, and I use the intent to know the info I need, before this change I retrieved the package, now I retrieve the component, but which is the right way to do it? Which is the right field to pass this information? I could just add the line intent.setPackage(chosen); back in the openModule and revert the fix of the incognito feature; or just leave it as it is now, with the fix.

@TrianguloY
Copy link
Owner

Then, in local I have already merged with master, but I had one issue that caught my attention, and I need help deciding which is the right way to solve it. So, in this commit (the URL puts you on the exact line, no need to search for it), the intent the app would send doesn't intent.setPackage(chosen) anymore and instead relies on intent.setComponent(chosen.getComponent). This is already fixed in my local branch, but, the incognito feature is separate from the app itself, and I use the intent to know the info I need, before this change I retrieved the package, now I retrieve the component, but which is the right way to do it? Which is the right field to pass this information? I could just add the line intent.setPackage(chosen); back in the openModule and revert the fix of the incognito feature; or just leave it as it is now, with the fix.

I may be wrong (if so just correct me) but from what I could find the component is more specific than a package. You can see the component as a package+activity (sort of). The change was to allow using different activities for the same application (aka package) #196 because otherwise only one was shown.
Using a component should be preferred, but if your logic only has an available package for some reason, replacing the component with a package only is ok, if it still works as expected.

In the commit you mention the exact change was splitted into two files, so it may be a bit hard to follow, but the important distinction is:

  • When asking android using the queryIntentActivities, it returns a list of ResolveInfo. This contains information about the exact component that can resolve the intent, including package and activity itself.
  • In the old version I was discarding everything and only extracting the packageName: packages.add(resolveInfo.activityInfo.packageName);, plus manually removing 'ourselves' (urlcheck) from the list.
  • In the new version I'm using the advanced queryIntentActivityOptions to do the 'except ourselves' directly but the important distinction is that now I'm returning the whole ResolveInfo (which I wrap in a custom IntentApp just for easier use, but it's just a simple wrapper): intentApps.add(new IntentApp(resolveInfo));

@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Oct 9, 2024

Using a component should be preferred, but if your logic only has an available package for some reason, replacing the component with a package only is ok, if it still works as expected.

It doesn't really matter, I just need to know the package and I'll modify everything needed.

Because the concept of the library is "give me a package and a mode (incognito) and I'll handle it for you" I think I should use intent.getPackage() and ensure the ComponentInfo is what I need it to be. In the future there might be some kind of special treatment if there are multiple activities in the same package that we can apply mode X, but currently that is not the case. So I'll be adding back the line .setPackage(chosen).

I'm going to add some info to the debug module too. It shouldn't take long but I'm busy until this weedend, hopefully by the end of the next one I have everything done and ready to merge!

Added info to the debug module
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Pmd (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@PabloOQ PabloOQ marked this pull request as ready for review October 20, 2024 15:08
@PabloOQ
Copy link
Collaborator Author

PabloOQ commented Oct 20, 2024

That should be it, no conflicts and finished! There are things that still need to be worked on, but for that I need feedback first, mainly when there are permissions needed but the user has not given those yet and wants to open an url in incognito.
Also the issue with foldables and the bubble, but that requires androidx, I will wait until this is an independent app to implement it.

Let me know if you need me to make fixes or anything, and don't worry if you take your time, I took mine too, I don't like at all how I dragged this for almost a whole year, my apologies.

@TrianguloY
Copy link
Owner

Understood! My turn then.
There are more than 2500 new lines of code, so yeah it may take me a while. I'll try to post here updates when needed, and I may ask you some clarifications if I need.

Also, I'm now in the middle of the automation implementation, which reorders slightly the existing incognito code (there will be an automation to enable incognito, but there are no changes related to the functionality itself). If I push that change first (and this pr reports conflicts) I'll fix them myself, don't worry about that (that will also help me understand the changes).

There are also two failing github actions, but I want to review them because I'm not sure if it's really an issue with the code or a misconfiguration of the action itself, so for now just ignore that.

Again, thank you for your effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Assets Changes some assets Core Change the core code Translation Changes for a non-english locale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants