Skip to content

Add support for Vivaldi snapshot browser#822

Merged
mpbw2 merged 2 commits intomasterfrom
vivaldi-snapshot
Apr 6, 2020
Merged

Add support for Vivaldi snapshot browser#822
mpbw2 merged 2 commits intomasterfrom
vivaldi-snapshot

Conversation

@mpbw2
Copy link
Contributor

@mpbw2 mpbw2 commented Apr 6, 2020

Add autofill (accessibility) support for Vivaldi snapshot browser

┆Issue is synchronized with this Asana task by Unito

@mpbw2 mpbw2 requested review from cscharf and kspearrin April 6, 2020 14:25
new Browser("jp.co.fenrir.android.sleipnir_black", "url_text"),
new Browser("jp.co.fenrir.android.sleipnir_test", "url_text"),
new Browser("com.vivaldi.browser", "url_bar"),
new Browser("com.vivaldi.browser.snapshot", "url_bar"),
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to add this to Autofill Helpers CompatBrowsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't appear to make a difference (neither vivaldi seems to work with autofill, at least for me). I can add it anyway if you'd like.

Copy link
Member

Choose a reason for hiding this comment

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

Might as well add it

@mpbw2 mpbw2 merged commit 8ad44b4 into master Apr 6, 2020
@mpbw2 mpbw2 deleted the vivaldi-snapshot branch April 6, 2020 15:43
@contribucious
Copy link

contribucious commented Apr 14, 2020

Hello,

@mportune-bw Is there a specific reason for not adding com.vivaldi.browser.snapshot to autofillservice.xml too? As was the case with com.vivaldi.browser when it was added?

@kspearrin More generally, is autofillservice.xml still used? If yes, why so many "package names" missing when compared to AutofillHelpers.cs? Namely:

# OK, these are special cases (because listed as working natively) ...
com.duckduckgo.mobile.android
org.mozilla.focus
org.mozilla.klar

# ... but concerning these cases? (listed as working using the compatibility shim)
com.android.browser
com.opera.touch
com.qwant.liberty
com.vivaldi.browser.snapshot
org.torproject.torbrowser

Thanks.

@kspearrin
Copy link
Member

@mportune-bw Can you evaluate if we still need to be using autofillservice.xml definitions?

@mpbw2
Copy link
Contributor Author

mpbw2 commented Apr 15, 2020

Looks like yes, if my understanding of the autofill service compatibility mode is correct. #834 brings it all back together. (Thanks for the pointer @contribucious )

@contribucious
Copy link

contribucious commented Apr 16, 2020

@mportune-bw No problem and thank you! 👍

@contribucious
Copy link

contribucious commented Apr 16, 2020

@kspearrin / @mportune-bw ⬇️

@mportune-bw Can you evaluate if we still need to be using autofillservice.xml definitions?

To maintain a list of browsers in compatibility mode, considering that Android requires the use of this dedicated XML file anyway, why not rather reason in the opposite direction therefore, @kspearrin?


PRESS ME TO READ MORE ABOUT THIS IDEA ... 👈

💡 Knowing that ...

  1. Data uniqueness is important (easier maintenance, no desynchronization between two same data sets possible / thus no more warning comment to put for future contributors, ...);
  2. We already have a perfect file which is: standardized (yay), dedicated (yay) and in a file format adapted to data (fluctuating data);

... and that you want to manage this data set from CompatBrowsers in your code, why not just populate it from this XML file, therefore? Below ... code for you on this subject! Feel free to do whatever you want with it, then!

:octocat: Code

Basic illustration

▶️ https://dotnetfiddle.net/RSDPuP (code will auto run)

Implementation (simple version: .FirstAttribute used)

▶️ https://dotnetfiddle.net/5LffXY (code will auto run)

Implementation (improved version: tag/attribute names used)

▶️ https://dotnetfiddle.net/jANQX4 (code will auto run)

👉 Edited multiple times to better comply with C# Coding Conventions (with some intentional exceptions). Last modified: April 20, 2020.

⚠️ Caution

For performance reasons, it might be good however to cache the extracted XML data for a certain period of time, using MemoryCache for example.

Otherwise, at each OnFillRequest in particular, the XML file will be reread and its content extracted and parsed again.

👉 Make sure, however, that the XML file will be reread (and data cached again) when a new version of it is there.

Whether it is:

  • manually (after a Bitwarden update);
  • automatically (Bitwarden version detected as having changed or otherwise: detection via metadata or checksum of the XML file itself);
  • automagically (advanced cache system, with file change detection mechanism).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants