Skip to content

fix: show consent dialog on firefox mobile#145

Closed
8bitgentleman wants to merge 1 commit intoActivityWatch:masterfrom
8bitgentleman:firefox-mobile-consent-dialog
Closed

fix: show consent dialog on firefox mobile#145
8bitgentleman wants to merge 1 commit intoActivityWatch:masterfrom
8bitgentleman:firefox-mobile-consent-dialog

Conversation

@8bitgentleman
Copy link
Copy Markdown

@8bitgentleman 8bitgentleman commented Jan 27, 2025

This is a fix for ActivityWatch/activitywatch#1115 where on firefox mobile the consent dialog was prevented from opening and therefore prevented from enabling the extension.

Previously chrome.windows.create() was used to show the dialog but according to the Firefox Android documentation, the windows API is not supported on Firefox for Android since there is only one Firefox window on Android, and the browser cannot open or manipulate additional browser windows.

On firefox mobile instead of opening a new page/window, we now embed the consent content directly in the window and only show/hide when requested.


Important

Fixes consent dialog issue on Firefox mobile by embedding consent content directly in the main window.

  • Behavior:
    • On Firefox mobile, embed consent content in popup.html instead of using chrome.windows.create().
    • showConsentDialog() in popup.js now checks isFirefoxAndroid() to determine dialog display method.
  • JavaScript:
    • Added isFirefoxAndroid() function in popup.js to detect Firefox on Android.
    • Updated domListeners() in popup.js to handle consent actions for Firefox Android.
  • HTML/CSS:
    • Added #consent-content div in popup.html for embedded consent content.
    • Updated styles in style.css for #consent-content and buttons.

This description was created by Ellipsis for da1f74f. It will automatically update as commits are pushed.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to da1f74f in 1 minute and 21 seconds

More details
  • Looked at 266 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. static/popup.html:71
  • Draft comment:
    Typo in the word 'wether'. It should be 'whether'.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The HTML file has a minor typo in the word 'whether'.
2. static/popup.js:77
  • Draft comment:
    Ensure consistent use of chrome or browser APIs. Mixing them can cause compatibility issues across different browsers.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The JavaScript file uses chrome and browser APIs interchangeably. This could lead to issues on different browsers.
3. static/style.css:81
  • Draft comment:
    The .consent-buttons class is not used in the HTML. Consider removing it if it's not needed.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The CSS file has a class .consent-buttons that is not used in the HTML. This might be a leftover or an oversight.

Workflow ID: wflow_zSTLa5S9VI3PL4RU


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@BelKed
Copy link
Copy Markdown
Contributor

BelKed commented Feb 6, 2025

@8bitgentleman May have been fixed by #152 as it now opens a new tab instead of a new window :)

@BelKed
Copy link
Copy Markdown
Contributor

BelKed commented Feb 25, 2025

It's fixed as reported by @adminvulcano in #24 (comment) :)

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