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

Use legacy User-Agent by default, add more settings #440

Closed
wants to merge 7 commits into from

Conversation

felipeerias
Copy link
Collaborator

Our User-Agent by default (when not in Desktop mode) becomes:

Mozilla/5.0 (Android 10; Mobile VR; rv:105.0) Gecko/105.0 Firefox/105.0

This can be modified through the settings:

  • use Wolvic's own UA: report ourselves as "Firefox" (the GeckoView default) or "Wolvic"
  • UA mode: "Mobile" or "Mobile VR"

User-Agent settings have been moved to the Developer section.

Add a new setting to decide whether to use Wolvic's own User-Agent or GeckoView's default one.

Add "Mobile" and "Mobile VR" variants to Wolvic's User-Agent, to match what GeckoView does.

Update the User-Agent when a page is reloaded, so changes to the settings can be effective immediately.

English, Spanish, and Galician translations.

Move the User-Agent settings to the Developer section.

Add a new setting to decide whether to use Wolvic's own User-Agent or GeckoView's default one.

Add "Mobile" and "Mobile VR" variants to Wolvic's User-Agent, to match what GeckoView does.

Update the User-Agent when a page is reloaded.

English, Spanish, and Galician translations.
@felipeerias felipeerias requested review from svillar and elima January 25, 2023 05:27
Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Looking good, there are a couple of things that I'd like to clarify first.

Also, we talked about showing a dialog warning the user everytime they select a UA which is not the default one (in this case Wolvic's). That information dialog should tell users that enabling Wolvic UI's might break some sites and encourage them to report those missbehaviours. We could talk offline about the exact text to be used.

public Point getDimensions() {
return new Point(WidgetPlacement.dpDimension(getContext(), R.dimen.settings_dialog_width),
WidgetPlacement.dpDimension(getContext(), R.dimen.developer_options_height));
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the relationship of this with anything else.

Is it something that was missing? Does it fix any particular issue? If that's the case it should go in a different PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes the dialog a bit taller, which is needed after adding the two settings for the User-Agent.

The alternative was to keep the dialog with the same height, but that looks worse:

08b9cbb2b6a8529c27325e8a27f43551

@Override
protected SettingViewType getType() {
return SettingViewType.LANGUAGE_VOICE;
return SettingViewType.DEVELOPER;
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a little issue that I saw when changing the dialog, it seems too small for its own PR but I can open one if needed.

@svillar
Copy link
Member

svillar commented Jan 25, 2023

We should also check which the exact UA we want to use by default, because DelightXR videos cannot be played unless you append a OculusBrowser/X declaration in my own experience

@felipeerias felipeerias requested a review from svillar January 26, 2023 10:06
@@ -76,7 +76,7 @@ public class Session implements WContentBlocking.Delegate, WSession.NavigationDe
private static UriOverride sDesktopModeOverrides;
private static final long KEEP_ALIVE_DURATION_MS = 1000; // 1 second.

private static final String WOLVIC_USER_AGENT_MOBILE = "Mozilla/5.0 (Android 10; Mobile; rv:105.0) Gecko/105.0 Wolvic/" + BuildConfig.VERSION_NAME;
private static final String WOLVIC_USER_AGENT_MOBILE = "Mozilla/5.0 (Android 10; Mobile; rv:105.0) Gecko/105.0 Wolvic/" + BuildConfig.VERSION_NAME + " Firefox/105.0 OculusBrowser/25.3.0.4.30.438623098 VR Safari/537.36";
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the back and forth, but it looks like we don't need that.

So we'll stick to the plan of having Gecko/x.x Firefox/x.x by default and then letting users to opt-in to Gecko/x.x Wolvic/x.x in developer options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Change reverted. This is how the User-Agent works now:

  • Not Wolvic's own UA, VR mode (default):
    Mozilla/5.0 (Android 10; Mobile VR; rv:105.0) Gecko/105.0 Firefox/105.0
  • Not Wolvic's own UA, Mobile mode:
    Mozilla/5.0 (Android 10; Mobile; rv:105.0) Gecko/105.0 Firefox/105.0
  • Wolvic's own UA, VR mode:
    Mozilla/5.0 (Android 10; Mobile VR; rv:105.0) Gecko/105.0 Wolvic/1.2
  • Wolvic's own UA, Mobile mode:
    Mozilla/5.0 (Android 10; Mobile; rv:105.0) Gecko/105.0 Wolvic/1.2

@svillar
Copy link
Member

svillar commented Jan 26, 2023

Also as I mentioned in other comment, we should add some dialog warning the user about enabling Wolvic UA because it might break some sites.

@felipeerias
Copy link
Collaborator Author

For reference, this is a related issue in Firefox Reality: MozillaReality/FirefoxReality#478

@felipeerias
Copy link
Collaborator Author

I have added an alert dialog when you use Wolvic's own User-Agent:

Warning

Changing the default "User-Agent" value is an experimental feature and might break some websites.

@svillar
Copy link
Member

svillar commented Jan 30, 2023

Superseeded by #453

@svillar svillar closed this Jan 30, 2023
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.

2 participants