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

GH-2188 - Adjustments for Ghostery Browser #622

Merged
merged 5 commits into from Oct 26, 2020
Merged
Changes from 1 commit
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

Next

update setting for Ghostery Browser

  • Loading branch information
christophertino committed Oct 22, 2020
commit 11d8b023f08bd972a0a4d106d2faea0bd95acc70
@@ -14,6 +14,9 @@
import React from 'react';
import { openSupportPage, openHubPage } from '../utils/msg';
import PanelToTabLink from './BuildingBlocks/PanelToTabLink';
import globals from '../../../src/classes/Globals';

const { BROWSER_INFO } = globals;

/**
* Render Help view that user can open from the header drop-down menu
@@ -23,9 +26,11 @@ const Help = () => (
<div className="row">
<div className="small-12 columns">
<h1>{t('panel_help_panel_header')}</h1>
<div className="support-section">
<a href="#" onClick={openHubPage}>{t('panel_help_setup')}</a>
</div>
{ BROWSER_INFO.name !== 'ghostery_desktop' && (
This conversation was marked as resolved by christophertino

This comment has been minimized.

@sammacbeth

sammacbeth Oct 23, 2020
Contributor

So we completely disable access to the hub page on the browser? Would it not be useful for some users who may want to access it later?

This comment has been minimized.

@christophertino

christophertino Oct 23, 2020
Author Member

We would need to adjust opt-in default settings for the hub but it's doable.

This comment has been minimized.

@christophertino

christophertino Oct 23, 2020
Author Member

Fixed

<div className="support-section">
<a href="#" onClick={openHubPage}>{t('panel_help_setup')}</a>
</div>
)}
<div className="support-section">
<h3>{t('panel_help_questions_header')}</h3>
<PanelToTabLink href="https://www.ghostery.com/faqs/">{t('panel_help_faq')}</PanelToTabLink>
@@ -3,10 +3,11 @@
"author": "Ghostery",
"name": "__MSG_name__",
"short_name": "Ghostery",
"version": "8.5.3",
"version_name": "8.5.3",
"version": "8.5.4",
"version_name": "8.5.4",
"default_locale": "en",
"description": "__MSG_short_description__",
"debug": true,
This conversation was marked as resolved by christophertino

This comment has been minimized.

@sammacbeth

sammacbeth Oct 23, 2020
Contributor

Should this be here?

This comment has been minimized.

@christophertino

christophertino Oct 23, 2020
Author Member

yea it's fine for develop. It gets removed by the builder.

"icons": {
"16": "app/images/icon16.png",
"48": "app/images/icon48.png",
@@ -114,4 +115,4 @@
"cliqz/offers-templates/checkout.html",
"cliqz/offers-templates/control-center.html"
]
}
}
@@ -1617,6 +1617,12 @@ function initializeGhosteryModules() {
conf.install_random_number = randomNumber;
conf.install_date = dateString;

// Set default search partners for Ghostery Desktop Browser. These can be removed
// by the user under Trusted Site settings.
if (BROWSER_INFO.name === 'ghostery_desktop') {
conf.site_whitelist.push('bing.com', 'search.yahoo.com', 'startpage.com');
}

metrics.setUninstallUrl();

metrics.ping('install');
@@ -1642,9 +1648,9 @@ function initializeGhosteryModules() {
conf.enable_ad_block = !adblocker.isDisabled;
conf.enable_anti_tracking = !antitracking.isDisabled;
conf.enable_human_web = !humanweb.isDisabled;
conf.enable_offers = !offers.isDisabled && !IS_ANDROID;
conf.enable_offers = !offers.isDisabled && !IS_ANDROID && BROWSER_INFO.name !== 'ghostery_desktop';
This conversation was marked as resolved by christophertino

This comment has been minimized.

@sammacbeth

sammacbeth Oct 23, 2020
Contributor

Is there a potential race here? How early is this block evaluated in the startup? The browser name will not be correct until after chrome.runtime.getBrowserInfo has resolved.

This comment has been minimized.

@christophertino

christophertino Oct 23, 2020
Author Member

This evaluates pretty far down the init chain and is nested inside cliqz.start(). The line directly below uses a similar check for android and has been working ok. But I admit I don't love this implementation. Open to suggestions.

This comment has been minimized.

@sammacbeth

sammacbeth Oct 23, 2020
Contributor

We could attach a Globals.browserInfoReady Promise in Globals when we call buildBrowserInfo, which we can then just await at these points that need to be sure that the browser check has been done.


if (IS_FIREFOX && BROWSER_INFO.name !== 'ghostery_android') {
if (IS_FIREFOX && BROWSER_INFO.name !== 'ghostery_desktop' && BROWSER_INFO.name !== 'ghostery_android') {
if (globals.JUST_INSTALLED) {
conf.enable_human_web = false;
conf.enable_offers = false;
@@ -1747,7 +1753,7 @@ function initializeGhosteryModules() {
// Open the Ghostery Hub on install with justInstalled query parameter set to true.
// We need to do this after running scheduledTasks for the first time
// because of an A/B test that determines which promo variant is shown in the Hub on install
if (globals.JUST_INSTALLED) {
if (globals.JUST_INSTALLED && BROWSER_INFO.name !== 'ghostery_desktop') {
const showAlternateHub = conf.hub_layout === 'alternate';
const route = showAlternateHub ? '#home' : '';
chrome.tabs.create({
@@ -111,8 +111,8 @@ class ConfData {
_initProperty('enable_click2play', true);
_initProperty('enable_click2play_social', true);
_initProperty('enable_human_web', !IS_CLIQZ && !IS_FIREFOX);
_initProperty('enable_metrics', false);
_initProperty('enable_offers', !IS_CLIQZ && !IS_FIREFOX && !IS_ANDROID);
_initProperty('enable_metrics', BROWSER_INFO.name === 'ghostery_desktop');
This conversation was marked as resolved by christophertino

This comment has been minimized.

@sammacbeth

sammacbeth Oct 23, 2020
Contributor

Again, could this be a race?

This comment has been minimized.

@christophertino

christophertino Oct 23, 2020
Author Member

Same as above, it does evaluate in order but the possibility for a race condition technically exists.

_initProperty('enable_offers', !IS_CLIQZ && !IS_FIREFOX && !IS_ANDROID && BROWSER_INFO.name !== 'ghostery_desktop');
_initProperty('enable_abtests', true);
_initProperty('enable_smart_block', true);
_initProperty('expand_all_trackers', true);
@@ -18,19 +18,6 @@ import parser from 'ua-parser-js';
const manifest = chrome.runtime.getManifest();
const isCliqzBrowser = !!(chrome.runtime.isCliqz);

/**
* Check for information about this browser
* Note: This is asynchronous and not available at runtime.
* @private
* @return boolean
*/
function _checkBrowserInfo() {
if (typeof chrome.runtime.getBrowserInfo === 'function') {
return chrome.runtime.getBrowserInfo();
}
return Promise.resolve(false);
}

/**
* Structure which holds parameters to be used throughout the code, a.k.a. global values.
* Most of them (but not all) are const.
@@ -203,7 +190,7 @@ class Globals {

// Check for Ghostery browsers
if (browser.includes('firefox') || browser.includes('mozilla')) {
_checkBrowserInfo().then((info) => {
Globals._checkBrowserInfo().then((info) => {
if (info.name === 'Ghostery') {
if (platform.includes('android')) {
this.BROWSER_INFO.displayName = 'Ghostery Android Browser';
@@ -220,6 +207,19 @@ class Globals {
});
}
}

/**
* Check for information about this browser
* Note: This is asynchronous and not available at runtime.
* @private
* @return boolean
*/
static _checkBrowserInfo() {
if (typeof chrome.runtime.getBrowserInfo === 'function') {
return chrome.runtime.getBrowserInfo();
}
return Promise.resolve(false);
}
}

// return the class as a singleton
ProTip! Use n and p to navigate between commits in a pull request.