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

Cross platform extension #569

Merged
merged 5 commits into from Jun 19, 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

Use new Geckoview page action popup support

Launches info popup in a temporary webview, rather than a full tab.
  • Loading branch information
sammacbeth committed Jun 16, 2020
commit bbdd570666f5f2294115088633b0b9ca6de4ba49
@@ -1341,11 +1341,8 @@ function getDataForGhosteryTab(callback) {
*/
function initializePopup() {
if (BROWSER_INFO.os === 'android') {
chrome.browserAction.onClicked.addListener((tab) => {
chrome.tabs.create({
url: chrome.extension.getURL(`app/templates/panel_android.html?tabId=${tab.id}`),
This conversation was marked as resolved by christophertino

This comment has been minimized.

@christophertino

christophertino Jun 18, 2020
Member

@sammacbeth Check out

const tabId = new URLSearchParams(window.location.search).get('tabId');
. The React app uses that tabId value to fetch data for the panel view.

Using setPopup will require bumping our min FF Android version to 57+, which is fine.

This comment has been minimized.

@sammacbeth

sammacbeth Jun 19, 2020
Author Contributor

I was wondering why the panel seems to be working despite this, and it seems that there is code to handle the tabId parameter not being provided:

// The 'getPanelData' message is never sent by the panel, which uses ports only since 8.3.2
// The message is still sent by panel-android and by the setup hub as of 8.4.0
if (name === 'getPanelData') {
if (!message.tabId) {
utils.getActiveTab((tab) => {
const data = panelData.get(message.view, tab);
callback(data);
});

This code to get the active tab could be moved into the panel code to ensure it knows which tab it refers to in case an undefined tabId introduces other issues.

This comment has been minimized.

@christophertino

christophertino Jun 19, 2020
Member

Good catch. @IAmThePan we can most likely remove that tabId lookup in Panel.jsx (see above)

This comment has been minimized.

@IAmThePan

IAmThePan Jun 19, 2020
Contributor

Noted. However, including the tabId in the URL enables Anri to open the panel (android or regular) in a webpage rather than the popup so he can do regression tests.
I will incorporate these changes in my feature/update-android branch. I will keep the code to open the tabId without the query parameter, but I will make sure the query parameter can be included if the use (tester, or developer) wants easy access to the panel.

This comment has been minimized.

active: true,
});
chrome.browserAction.setPopup({
popup: 'app/templates/panel_android.html',
});
} else {
chrome.browserAction.setPopup({
ProTip! Use n and p to navigate between commits in a pull request.