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

Added support for firefox on android #1516

Merged
merged 22 commits into from
Aug 20, 2017
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions src/js/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,9 @@ function Badger() {
}

// Show icon as page action for all tabs that already exist
chrome.windows.getAll({populate: true}, function (windows) {
for (var i = 0; i < windows.length; i++) {
for (var j = 0; j < windows[i].tabs.length; j++) {
badger.refreshIconAndContextMenu(windows[i].tabs[j]);
}
chrome.tabs.query({}, function (tabs) {
for (var i = 0; i < tabs.length; i++) {
badger.refreshIconAndContextMenu(tabs[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}
});

Expand All @@ -68,6 +66,10 @@ function Badger() {
badger.INITIALIZED = true;
});

// Temporary feature check for firefox android while it doesn't support the full browserAction API
// Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1330159
this.isFirefoxMobile = !(chrome.browserAction.setBadgeText && chrome.browserAction.setPopup && chrome.browserAction.getPopup);

/**
* WebRTC availability check
*/
Expand Down Expand Up @@ -531,6 +533,10 @@ Badger.prototype = {
* @param {Integer} tabId chrome tab id
*/
updateBadge: function(tabId){
if(this.isFirefoxMobile){
return;
}

if (!this.showCounter()){
chrome.browserAction.setBadgeText({tabId: tabId, text: ""});
return;
Expand Down Expand Up @@ -602,7 +608,7 @@ Badger.prototype = {
}
return true;
},

/**
* Check if privacy badger is disabled, take an origin and
* check against the disabledSites list
Expand Down Expand Up @@ -744,7 +750,7 @@ Badger.prototype = {
* @param {Object} tab The tab to set the badger icon for
*/
refreshIconAndContextMenu: function (tab) {
if (!tab) {
if (!tab || this.isFirefoxMobile) {
return;
}

Expand All @@ -762,7 +768,6 @@ Badger.prototype = {
}

chrome.browserAction.setIcon({tabId: tab.id, path: iconFilename});
chrome.browserAction.setTitle({tabId: tab.id, title: "Privacy Badger"});
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, now this is localized properly by the "default_title" key in the manifest

},

};
Expand All @@ -776,12 +781,45 @@ function startBackgroundListeners() {
}
}, {urls: ["http://*/*", "https://*/*"]}, []);

if(badger.isFirefoxMobile) {
Copy link
Contributor

@cowlicks cowlicks Aug 15, 2017

Choose a reason for hiding this comment

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

Could you refactor these to be like:

if (!isMobile) {
  startRegularListeners(); // with an exposed named function
} else {
 startMobileListeners();
}

This way the mobile logic is separate from the regular logic. So we're able to test them independently .

// keeps track of popup id while one is open
var openPopupId = false;
var popup_url = chrome.runtime.getManifest().browser_action.default_popup;

// fake a popup
chrome.browserAction.onClicked.addListener(() => {
chrome.tabs.query({active: true, lastFocusedWindow: true}, (tabs) => {
var url = popup_url + "?tabId=" + tabs[0].id;
chrome.tabs.create({url, index: tabs[0].index + 1}, (tab) => {
openPopupId = tab.id;
});
});
});

// remove the 'popup' when another tab is activated
chrome.tabs.onActivated.addListener((activeInfo) => {
if(openPopupId != false && openPopupId != activeInfo.tabId) {
chrome.tabs.remove(openPopupId, () => {
openPopupId = false;
});
}
});
}

// Update icon if a tab changes location
chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab) {
if(changeInfo.status == "loading") {
badger.refreshIconAndContextMenu(tab);
}

if(badger.isFirefoxMobile && tab.url && openPopupId == tabId){
var new_url = new URL(tab.url);

// forget the popup id, because the user as overwritten the url
if(new_url.origin + new_url.pathname != popup_url){
openPopupId = false;
}
}
});

// Update icon if a tab is replaced or loaded from cache
Expand Down
8 changes: 8 additions & 0 deletions src/js/popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,14 @@ function syncUISelections() {
* seems to be that it is synchronous.
*/
function getTab(callback) {
// Temporary fix for Firefox Android while it doesn't support `browser_action.default_popup`
if(badger.isFirefoxMobile){
chrome.tabs.query({active: true, lastFocusedWindow: true}, function(focusedTab) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can use chrome.tabs.getCurrent here instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getCurrent unfortunately doesn't work

chrome.tabs.get(parseInt(new URL(focusedTab[0].url).searchParams.get('tabId')), function(t) { callback(t); });
});
return;
}

chrome.tabs.query({active: true, lastFocusedWindow: true}, function(t) { callback(t[0]); });
}

Expand Down
1 change: 1 addition & 0 deletions src/skin/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<head>
<meta name="google" content="notranslate">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link type="text/css" href="/lib/vendor/jquery-ui-1.12.1.custom/jquery-ui.structure.min.css" rel="stylesheet" />
<link type="text/css" href="/lib/vendor/jquery-ui-1.12.1.custom/jquery-ui.theme.min.css" rel="stylesheet" />
<link type="text/css" media="screen" href="/skin/popup.css" rel="stylesheet" />
Expand Down
25 changes: 16 additions & 9 deletions src/skin/popup.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,21 @@
/*csslint ids:ignore*/

body {
min-width: 200px;
max-width: 400px;
margin: 0;
min-width: 430px; /* Chrome */
max-width: 100%; /* FF android */
font-size: 12px;
background: #fefefe;
color: #333;
font-family: helvetica, arial, sans-serif;
padding-left: 7px;
padding-right: 7px;
overflow-y: hidden;
overflow-x: hidden;
width: 400px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

creates a whitespace within chrome, use min-width instead

padding: 8px 15px;
box-sizing: border-box;
}
@media screen and (min--moz-device-pixel-ratio:0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

firefox specific css, otherwise the popup would render too wide

body{
min-width: 200px; /* FF android */
width: 430px; /* FF desktop */
}
}

a { text-decoration: none }
Expand Down Expand Up @@ -190,11 +194,13 @@ font-size: 16px;
max-height: 290px;
overflow-y: auto;
background-color: #E8E9EA;
position: relative;
}
.key {
position: fixed;
position: absolute;
height: 25px;
width: 235px;
left: 0;
right: 0;
z-index: 30;
background: #fefefe;
padding-top: 4px;
Expand Down Expand Up @@ -296,6 +302,7 @@ font-size: 16px;
z-index: -1;
opacity: 0;
padding: 3%;
box-sizing: border-box;
color: #ccc;
background: url("/skin/background.png");
transition: opacity .1s ease;
Expand Down