Create right-click menu

[fkneist]

2nd try, because of messed up git history in last PR

[max-baz]

Btw that reminds me of an idea that I had: what would you think about porting the project to TypeScript, then part of the documentation would become unnecessary and wouldn't need to be manually maintained + it would maybe be a bit easier to understand input/output parameters quickly.

Extracting the question for visibility. @erayd is our main frontend person so I'll let him properly answer this, I can say this was considered back when we created the project, but also I'm led to believe that typescript has improved a lot since 2018, so maybe it's good to spend at least 10 minutes and reconsider again, even if we end up deciding to stick with javascript.

Ideally it would be good for him to also review this PR, but he is busy in the next couple of weeks and you are eager to contribute and we all appreciate that and I want to support you, so let's agree that I'll test it later today and if everything works well I'll merge, so that you can continue working on the next improvements, and we might have to fix some additional code review comments in a separate PR later when @erayd gets time to look at your contributions? 😉

[fkneist]

Another thing I noticed while testing is, that the order of the child entries is not always the same (multiple accounts per host). I think in the popup this is intended, as to move frequently used entries up, but in the right-click menu I find it a bit confusing. What would you guys think of alphabetical sorting in the right-click menu?

[erayd]

@fkneist

What would you guys think of alphabetical sorting in the right-click menu?

I think it would be great for instances where there are only a small number of entries - but where there are large numbers, we should stick to frequency. As an example, I know someone who has over 200 microsoft.com accounts in their password manager - in that kind of scenario, alphabetised sorting would render the right-click menu unusable.

[fkneist]

Hi there, it's been a while :smiley:

I had some time and tinkered on the solution in the last few days:

• Not using chrome.contextMenus.removeAll() anymore, rather switching visibility of parent item and remove child items on demand one by one.
• Credentials are cached per tab with a TTL now
• Unfortunately the solution feels way longer, in terms of LOC, now, probably more surface area for bugs :unamused:

Some scenarios I tested:

• Visit a page that I know I have credentials for. Also the opposite.
• Visit a page that I don't have credentials for after I visited one that I have credentials for. And vice versa.
• Visit a internal browser page and then above scenarios. And vice versa

Known limitation:

• when I have two tabs open, site A and site B. I site A is a slow site and I press refresh and quickly change to site B, it can happen even though I have credentials for site B that the context menu doesn't show for a short time

Let me know what you think @maximbaz and @erayd

[max-baz]

Lovely, thanks for your time and contribution!

There are a couple of other items we need to deal with first (e.g. #290 has a lot of work that's been put in and is very close to being merged, #320 has a deadline in June or the extension will be delisted from Chrome Web Store), so I think it's most realistic that we'll look at this PR afterwards. But if we have the time to add additional items in the release, I would love this PR to be one of them!