-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Moved UnicodeFontHandler to background scripts #11
Conversation
tdulcet
commented
Nov 29, 2020
- Moved UnicodeFontHandler to background scripts. Closes Code style: Move UnicodeFontHandler to background scripts #6.
- Minor update to README. Should have been in Added Unicode autocorrect and font conversion #2
@@ -30,7 +30,7 @@ See: | |||
|
|||
## Features | |||
* Puts your privacy first! Privacy is the default here. | |||
* Follows the [Firefox Photon Design](https://design.firefox.com/photon). | |||
* Follows the [Firefox](https://design.firefox.com/photon) and [Thunderbird](https://style.thunderbird.net/) Photon Design. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice, let’s hope these design systems don’t diverge too much… 🙃
@@ -207,6 +207,7 @@ function applySettings(unicodeFont) { | |||
* @returns {void} | |||
*/ | |||
export async function init() { | |||
// Remove once https://bugzilla.mozilla.org/show_bug.cgi?id=1595822 is fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this comment comments a await isMobile()
exclusion, but your bug is about Thunderbird?
How is this related?
isMobile
AFAIK should only check (and in this case exclude) Firefox for Android…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that bug is about implementing the WebExtension context menu API in Firefox for Android.
@@ -87,6 +87,7 @@ <h1 data-i18n="">Unicode autocorrection</h1> | |||
</ul> | |||
</section> | |||
|
|||
<!-- Remove "mobile-incompatible" once https://bugzilla.mozilla.org/show_bug.cgi?id=1595822 is fixed --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here…
Do you “abuse” this mobile-incompatible blacklisting for Thunderbird? 🙃
If so, that’s of course not the proper way to do so. Because this add-on may (as for the auto-correction part, at least), run fine on Firefox for Android (Fenix, if they allow all add-ons in a future version). So we need a different thunderbird-incompatible
CSS and isThunderbird
JS or so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above. The context menu does not currently work in the compose window of Thunderbird because of bug 1656506, although it does not need to be disabled and it looks like this may be fixed by bug 1670832.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry about the comments, seems I was not fully awake when reviewing this. Of course your added comments are good and point out the FF for Android (Fenix) issues. (Though one might need to re-create these issues at https://github.com/mozilla-mobile/fenix/ so they see it. But they work on add-on compatibility in general anyway, already, so I see no need here.)
It seems I read Thunderbird and then applied everything I saw to Thunderbird 🙃 Sorry. It's all right, of course.